chore(refactor): update dependencies and return errors from verify (#405

) * refactor(dry): verifyOptions handles error string building * chore: yarn deduplicate * chore: update dev deps * chore: lint fixes
tophat · Jun 13, 2021 · 5fb1a8e · 5fb1a8e
1 parent d67bcb0
commit 5fb1a8e
Show file tree

Hide file tree

Showing 15 changed files with 2,293 additions and 2,677 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -6,7 +6,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node: ['12', '14']
+        node: ['12', '14', '16']
     steps:
       - uses: actions/checkout@v2
       - name: Install Node
@@ -30,7 +30,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node: ['12', '14']   
+        node: ['12', '14', '16']
     steps:
       - uses: actions/checkout@v2
       - name: Install Node
@@ -56,7 +56,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node: ['12', '14']   
+        node: ['12', '14', '16']
     steps:
       - uses: actions/checkout@v2
       - name: Install Node
@@ -89,7 +89,7 @@ jobs:
       - name: Install Node
         uses: actions/setup-node@v2
         with:
-          node-version: 14
+          node-version: 16
       - name: Yarn Cache
         id: yarn-cache
         uses: actions/cache@v2
@@ -114,7 +114,7 @@ jobs:
       - name: Install Node
         uses: actions/setup-node@v2
         with:
-          node-version: 14
+          node-version: 16
       - name: Yarn Cache
         id: yarn-cache
         uses: actions/cache@v2
@@ -139,7 +139,7 @@ jobs:
       - name: Install Node
         uses: actions/setup-node@v2
         with:
-          node-version: 14
+          node-version: 16
       - name: Yarn Cache
         id: yarn-cache
         uses: actions/cache@v2

diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,3 @@
-node_modules
-artifacts
+node_modules/
+artifacts/
+yarn-error.log
diff --git a/.husky/.gitignore b/.husky/.gitignore
@@ -0,0 +1 @@
+_
diff --git a/.husky/commit-msg b/.husky/commit-msg
@@ -0,0 +1,4 @@
+#!/bin/sh
+. "$(dirname "$0")/_/husky.sh"
+
+yarn commitlint --edit "$1"
diff --git a/.husky/pre-commit b/.husky/pre-commit
@@ -0,0 +1,4 @@
+#!/bin/sh
+. "$(dirname "$0")/_/husky.sh"
+
+yarn lint-staged
diff --git a/.nvmrc b/.nvmrc
@@ -1 +1 @@
-12.10.0
+16.3.0
diff --git a/package.json b/package.json
@@ -8,39 +8,39 @@
   "license": "Apache-2.0",
   "private": false,
   "dependencies": {
-    "web-ext": "^5.0.0"
+    "web-ext": "^5.0.0||^6.0.0"
   },
   "peerDependencies": {
     "aggregate-error": "^3.0.0",
     "semantic-release": "^16.0.0||^17.0.0"
   },
   "devDependencies": {
-    "@babel/core": "^7.6.0",
-    "@babel/preset-env": "^7.6.0",
-    "@tophat/commitizen-adapter": "^0.5.0",
-    "@tophat/commitlint-config": "^0.3.0",
-    "@tophat/eslint-config": "^0.8.0",
-    "@types/jest": "^26.0.0",
-    "aggregate-error": "^3.0.1",
-    "all-contributors-cli": "^6.9.1",
-    "babel-eslint": "^10.0.2",
-    "codecov": "^3.5.0",
-    "commitizen": "^4.0.3",
-    "commitlint": "^12.0.1",
-    "eslint": "^7.1.0",
-    "eslint-config-prettier": "^8.0.0",
-    "eslint-plugin-import": "^2.19.1",
-    "eslint-plugin-jest": "^24.0.0",
-    "eslint-plugin-prettier": "^3.1.0",
-    "husky": "^5.0.9",
-    "jest": "^26.0.1",
-    "jest-mock-module": "^0.1.3",
-    "lint-staged": "^10.0.1",
-    "memfs": "^3.0.3",
-    "prettier": "^2.0.1",
-    "semantic-release": "^17.0.1",
+    "@babel/core": "^7.14.5",
+    "@babel/preset-env": "^7.14.5",
+    "@tophat/commitizen-adapter": "^0.5.4",
+    "@tophat/commitlint-config": "^0.3.4",
+    "@tophat/eslint-config": "^0.9.0",
+    "@types/jest": "^26.0.23",
+    "aggregate-error": "^3.1.0",
+    "all-contributors-cli": "^6.20.0",
+    "babel-eslint": "^10.1.0",
+    "codecov": "^3.8.2",
+    "commitizen": "^4.2.4",
+    "commitlint": "^12.1.4",
+    "eslint": "^7.28.0",
+    "eslint-config-prettier": "^8.3.0",
+    "eslint-plugin-import": "^2.23.4",
+    "eslint-plugin-jest": "^24.3.6",
+    "eslint-plugin-prettier": "^3.4.0",
+    "husky": "^6.0.0",
+    "jest": "^27.0.4",
+    "jest-mock-module": "^0.1.4",
+    "lint-staged": "^11.0.0",
+    "memfs": "^3.2.2",
+    "prettier": "^2.3.1",
+    "semantic-release": "^17.4.3",
     "unionfs": "^4.4.0",
-    "yarn-deduplicate": "^3.0.0"
+    "yarn-deduplicate": "^3.1.0"
   },
   "scripts": {
     "build": "mkdir -p artifacts; echo 'When changing this remember to update @semantic-release/npm.'",
@@ -74,12 +74,6 @@
       "yarn lock-check"
     ]
   },
-  "husky": {
-    "hooks": {
-      "commit-msg": "commitlint -E HUSKY_GIT_PARAMS",
-      "pre-commit": "lint-staged"
-    }
-  },
   "jest": {
     "setupFilesAfterEnv": [
       "./tests/setup.js"

diff --git a/src/prepare.js b/src/prepare.js
@@ -3,8 +3,10 @@ const path = require('path')
 
 const { verifyOptions } = require('./utils')
 
+const withErrMsg = (e, message) => ((e.message = `${message}. ${e.message}`), e)
+
 const prepare = (options, { nextRelease, logger }) => {
-    const { sourceDir, manifestPath } = verifyOptions(options)
+    const { sourceDir, manifestPath } = verifyOptions(options).verified
 
     const version = nextRelease.version
     const normalizedManifestPath = path.join(sourceDir, manifestPath)
@@ -13,23 +15,21 @@ const prepare = (options, { nextRelease, logger }) => {
     try {
         manifest = fs.readFileSync(normalizedManifestPath)
     } catch (e) {
-        throw new Error('Unable to read manifest.json file from dist folder')
+        throw withErrMsg(e, 'Failed to read manifest file from dist folder')
     }
 
     try {
         const jsonManifest = JSON.parse(manifest.toString())
         jsonManifest.version = version
         manifest = JSON.stringify(jsonManifest, null, 2)
     } catch (e) {
-        throw new Error('Failed to parse manifest.json into JSON')
+        throw withErrMsg(e, 'Failed to parse manifest into JSON')
     }
 
     try {
         fs.writeFileSync(normalizedManifestPath, manifest)
     } catch (e) {
-        throw new Error(
-            'Failed to write updated manifest.json to the dist folder',
-        )
+        throw withErrMsg(e, 'Failed to write updated manifest to dist folder')
     }
 
     logger.log('Wrote version %s to %s', version, normalizedManifestPath)

diff --git a/src/publish.js b/src/publish.js
@@ -4,7 +4,7 @@ const path = require('path')
 const webExt = require('web-ext')
 const { signAddon: defaultAddonSigner } = require('sign-addon')
 
-const { allowedChannels } = require('./constants')
+const { allowedChannels, requiredOptions } = require('./constants')
 const { verifyOptions } = require('./utils')
 
 const publish = async (options) => {
@@ -15,13 +15,8 @@ const publish = async (options) => {
     // If there's an error with the validation, webExt sign will log a link to
     // the console which will lead to the validation page which should contain
     // detailed reasons why the extension was rejected
-    const {
-        extensionId,
-        artifactsDir,
-        channel,
-        sourceDir,
-        targetXpi,
-    } = verifyOptions(options, ['extensionId', 'targetXpi'])
+    const { extensionId, artifactsDir, channel, sourceDir, targetXpi } =
+        verifyOptions(options, requiredOptions).verified
 
     const { FIREFOX_API_KEY, FIREFOX_SECRET_KEY } = process.env
 

diff --git a/src/utils.js b/src/utils.js
@@ -8,16 +8,19 @@ const maybeThrowErrors = (errors) => {
     }
 }
 
-const verifyOptions = (options, required = []) => {
+const verifyOptions = (options, required = {}, throwErrors = true) => {
     const errors = []
     const mergedOptions = { ...defaultOptions, ...options }
-    required.forEach((prop) => {
+    Object.keys(required).forEach((prop) => {
+        mergedOptions[prop] = options[prop]
         if (mergedOptions[prop] === undefined) {
-            errors.push(`${prop} is missing from the options`)
+            errors.push(`${prop} is missing. ${required[prop]}`)
         }
     })
-    maybeThrowErrors(errors)
-    return mergedOptions
+    if (throwErrors) {
+        maybeThrowErrors(errors)
+    }
+    return { verified: mergedOptions, errors }
 }
 
 module.exports = {

diff --git a/src/verifyConditions.js b/src/verifyConditions.js
@@ -5,26 +5,9 @@ const { requiredEnvs, requiredOptions } = require('./constants')
 const { maybeThrowErrors, verifyOptions } = require('./utils')
 
 const verifyConditions = (options) => {
-    const verified = verifyOptions(options)
+    const { verified, errors } = verifyOptions(options, requiredOptions, false)
+    errors.push(...verifyOptions(process.env, requiredEnvs, false).errors)
     const { manifestPath, sourceDir } = verified
-    const errors = []
-
-    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(

diff --git a/tests/prepare.test.js b/tests/prepare.test.js
@@ -46,22 +46,22 @@ describe('prepare', () => {
 
     it('fails if cannot read manifest file', () => {
         expect(() => prepare(mockOptions, defaultConfig)).toThrow(
-            'Unable to read manifest.json file',
+            'Failed to read manifest',
         )
     })
 
     it('fails if cannot parse manifest file', () => {
         vol.fromJSON({ 'dist/manifest.json': 'this is not valid json' })
         expect(() => prepare({}, defaultConfig)).toThrow(
-            'Failed to parse manifest.json',
+            'Failed to parse manifest',
         )
     })
 
     it('fails if cannot update manifest file', () => {
         jest.spyOn(fs, 'readFileSync').mockImplementationOnce(() => '{}')
         expect(() =>
             prepare({ sourceDir: 'sourceDir' }, defaultConfig),
-        ).toThrow('Failed to write updated manifest.json')
+        ).toThrow('Failed to write updated manifest')
     })
 
     it.each([

diff --git a/tests/publish.test.js b/tests/publish.test.js
@@ -35,10 +35,8 @@ describe('publish', () => {
     })
     beforeEach(() => {
         vol.fromJSON({
-            [path.join(
-                mockOptions.sourceDir,
-                mockOptions.manifestPath,
-            )]: JSON.stringify(mockManifestJSON),
+            [path.join(mockOptions.sourceDir, mockOptions.manifestPath)]:
+                JSON.stringify(mockManifestJSON),
         })
     })
     afterEach(() => {

diff --git a/tests/verifyConditions.test.js b/tests/verifyConditions.test.js
@@ -39,13 +39,13 @@ describe('verifyConditions', () => {
 
     it('fails if extensionId is missing from options', () => {
         expect(() => verifyConditions({ targetXpi })).toThrow(
-            'No extensionId was specified',
+            'extensionId is missing',
         )
     })
 
     it('fails if targetXpi is missing from options', () => {
         expect(() => verifyConditions({ extensionId })).toThrow(
-            'No targetXpi was specified',
+            'targetXpi is missing',
         )
     })