fix: show more info on required options and env errors (#39)

* refactor: explicitly callout required config and env

* cr: remove duplication

* refactor: add more information to env error message

src/constants.js

@@ -0,0 +1,24 @@
+const defaultOptions = {
+    artifactsDir: './artifacts',
+    channel: 'unlisted',
+    manifestPath: 'manifest.json',
+    sourceDir: 'dist',
+}
+
+const requiredOptions = {
+    extensionId:
+        'Omitting this would create a new extension instead of a new version.',
+    targetXpi:
+        'Omitting this would leave the xpi file unnamed when it is returned from mozilla.',
+}
+
+const requiredEnvs = {
+    FIREFOX_API_KEY: 'Firefox api key used in webext sign command.',
+    FIREFOX_SECRET_KEY: 'Firefox api secret used in webext signing.',
+}
+
+module.exports = {
+    defaultOptions,
+    requiredEnvs,
+    requiredOptions,
+}

src/prepare.js

@@ -1,10 +1,10 @@
 const fs = require('fs')
 const path = require('path')
 
-const { verifyConfig } = require('./utils')
+const { verifyOptions } = require('./utils')
 
 const prepare = (options, { nextRelease, logger }) => {
-    const { sourceDir, manifestPath } = verifyConfig(options)
+    const { sourceDir, manifestPath } = verifyOptions(options)
 
     const version = nextRelease.version
     const normalizedManifestPath = path.join(sourceDir, manifestPath)

src/publish.js

@@ -3,7 +3,7 @@ const path = require('path')
 
 const webExt = require('web-ext').default
 
-const { verifyConfig } = require('./utils')
+const { verifyOptions } = require('./utils')
 
 const publish = async options => {
     // This will create an unsigned xpi from sourceDir folder (dist) it will
@@ -19,7 +19,7 @@ const publish = async options => {
         channel,
         sourceDir,
         targetXpi,
-    } = verifyConfig(options, ['extensionId', 'targetXpi'])
+    } = verifyOptions(options, ['extensionId', 'targetXpi'])
 
     const { FIREFOX_API_KEY, FIREFOX_SECRET_KEY } = process.env
     const { success, downloadedFiles } = await webExt.cmd.sign({

src/utils.js

@@ -1,19 +1,14 @@
 const AggregateError = require('aggregate-error')
 
+const { defaultOptions } = require('./constants')
+
 const maybeThrowErrors = errors => {
     if (errors.length > 0) {
         throw new AggregateError(errors)
     }
 }
 
-const defaultOptions = {
-    artifactsDir: './artifacts',
-    channel: 'unlisted',
-    manifestPath: 'manifest.json',
-    sourceDir: 'dist',
-}
-
-const verifyConfig = (options, required = []) => {
+const verifyOptions = (options, required = []) => {
     const errors = []
     const mergedOptions = { ...defaultOptions, ...options }
     required.forEach(prop => {
@@ -27,5 +22,5 @@ const verifyConfig = (options, required = []) => {
 
 module.exports = {
     maybeThrowErrors,
-    verifyConfig,
+    verifyOptions,
 }

src/verifyConditions.js

@@ -1,34 +1,34 @@
 const fs = require('fs')
 const path = require('path')
 
-const { maybeThrowErrors, verifyConfig } = require('./utils')
+const { requiredEnvs, requiredOptions } = require('./constants')
+const { maybeThrowErrors, verifyOptions } = require('./utils')
 
 const verifyConditions = options => {
-    const verified = verifyConfig(options)
-    const { extensionId, sourceDir, manifestPath, targetXpi } = verified
-    const { FIREFOX_API_KEY, FIREFOX_SECRET_KEY } = process.env
-
+    const verified = verifyOptions(options)
+    const { manifestPath, sourceDir } = verified
     const errors = []
-    if (!FIREFOX_API_KEY) {
-        errors.push('FIREFOX_API_KEY is missing from the environment')
-    }
-    if (!FIREFOX_SECRET_KEY) {
-        errors.push('FIREFOX_SECRET_KEY is missing from the environment')
-    }
-    if (!extensionId) {
-        errors.push(
-            'No extensionId was specified in package.json, this would create a new extension instead of a new version.',
-        )
-    }
-    if (!targetXpi) {
-        errors.push(
-            'No targetXpi was specified in package.json, this would leave the xpi file unnamed when it is returned from mozilla.',
-        )
-    }
+
+    Object.keys(requiredEnvs).forEach(envVarName => {
+        if (!process.env[envVarName]) {
+            errors.push(
+                `${envVarName} is missing from the environment. ${requiredEnvs[envVarName]}`,
+            )
+        }
+    })
+
+    Object.keys(requiredOptions).forEach(option => {
+        if (!verified[option]) {
+            errors.push(
+                `No ${option} was specified in package.json. ${requiredOptions[option]}`,
+            )
+        }
+    })
+
     const manifestExists = fs.existsSync(path.join(sourceDir, manifestPath))
     if (!manifestExists) {
         errors.push(
-            `${manifestPath} was not found in ${sourceDir}, dist folder needs to exist to run`,
+            `${manifestPath} was not found in ${sourceDir}, path does not exist.`,
         )
     }
     maybeThrowErrors(errors)