From 97af9a83c185f7170d2361c356a743635da11139 Mon Sep 17 00:00:00 2001
From: Joe Bottigliero <694253+jbottigliero@users.noreply.github.com>
Date: Sat, 25 Jan 2020 13:00:31 -0600
Subject: [PATCH] fix: Ensures provided `packageFiles` arguments are merged
 with `bumpFiles` when no `bumpFiles` argument is specified (default).

- Merges `packageFiles` with `defaults.bumpFiles` before merging in arguments when a `packageFile` argument is present.
- Adds some simple "validation" to updaters and improves error messaging.
- Adds a test case covering #533

closes #533
---
 index.js                       |  8 ++++++++
 lib/updaters/index.js          | 23 ++++++++++++++++++-----
 test/core.spec.js              | 26 ++++++++++++++++++++++++++
 test/mocks/manifest-6.3.1.json |  3 +++
 4 files changed, 55 insertions(+), 5 deletions(-)
 create mode 100644 test/mocks/manifest-6.3.1.json

diff --git a/index.js b/index.js
index 3a35555fc..56f3106b3 100755
--- a/index.js
+++ b/index.js
@@ -37,10 +37,18 @@ module.exports = async function standardVersion (argv) {
     throw Error(`custom changelog header must not match ${changelog.START_OF_LAST_RELEASE_PATTERN}`)
   }
 
+  /**
+   * If an argument for `packageFiles` provided, we include it as a "default" `bumpFile`.
+   */
+  if (argv.packageFiles) {
+    defaults.bumpFiles = defaults.bumpFiles.concat(argv.packageFiles)
+  }
+
   const args = Object.assign({}, defaults, argv)
   let pkg
   for (const packageFile of args.packageFiles) {
     const updater = resolveUpdaterObjectFromArgument(packageFile)
+    if (!updater) return
     const pkgPath = path.resolve(process.cwd(), updater.filename)
     try {
       const contents = fs.readFileSync(pkgPath, 'utf8')
diff --git a/lib/updaters/index.js b/lib/updaters/index.js
index 1e686301d..ec561a81e 100644
--- a/lib/updaters/index.js
+++ b/lib/updaters/index.js
@@ -26,7 +26,7 @@ function getUpdaterByFilename (filename) {
   )
 }
 
-function getCustomUpdater (updater) {
+function getCustomUpdaterFromPath (updater) {
   if (typeof updater === 'string') {
     return require(path.resolve(process.cwd(), updater))
   }
@@ -39,32 +39,45 @@ function getCustomUpdater (updater) {
   throw new Error('Updater must be a string path or an object with readVersion and writeVersion methods')
 }
 
+/**
+ * Simple check to determine if the object provided is a compatible updater.
+ */
+function isValidUpdater (obj) {
+  return (
+    typeof obj.readVersion === 'function' &&
+    typeof obj.writeVersion === 'function'
+  )
+}
+
 module.exports.resolveUpdaterObjectFromArgument = function (arg) {
   /**
    * If an Object was not provided, we assume it's the path/filename
    * of the updater.
    */
   let updater = arg
+  if (isValidUpdater(updater)) {
+    return updater
+  }
   if (typeof updater !== 'object') {
     updater = {
       filename: arg
     }
   }
   try {
-    if (updater.updater) {
-      updater.updater = getCustomUpdater(updater.updater)
+    if (typeof updater.updater === 'string') {
+      updater.updater = getCustomUpdaterFromPath(updater.updater)
     } else if (updater.type) {
       updater.updater = getUpdaterByType(updater.type)
     } else {
       updater.updater = getUpdaterByFilename(updater.filename)
     }
   } catch (err) {
-    if (err.code !== 'ENOENT') console.warn(err.message)
+    if (err.code !== 'ENOENT') console.warn(`Unable to obtain updater for: ${JSON.stringify(arg)}\n - Error: ${err.message}\n - Skipping...`)
   }
   /**
    * We weren't able to resolve an updater for the argument.
    */
-  if (!updater.updater) {
+  if (!isValidUpdater(updater.updater)) {
     return false
   }
 
diff --git a/test/core.spec.js b/test/core.spec.js
index 22d2b8e2c..b54515a56 100644
--- a/test/core.spec.js
+++ b/test/core.spec.js
@@ -568,6 +568,32 @@ describe('standard-version', function () {
     })
   })
 
+  it('`packageFiles` are bumped along with `bumpFiles` defaults [standard-version#533]', async function () {
+    mock({
+      bump: 'minor',
+      fs: {
+        '.gitignore': '',
+        'package-lock.json': JSON.stringify({ version: '1.0.0' }),
+        'manifest.json': fs.readFileSync('./test/mocks/manifest-6.3.1.json')
+      },
+      tags: ['v1.0.0']
+    })
+
+    await exec({
+      silent: true,
+      packageFiles: [
+        {
+          filename: 'manifest.json',
+          type: 'json'
+        }
+      ]
+    })
+
+    JSON.parse(fs.readFileSync('manifest.json', 'utf-8')).version.should.equal('6.4.0')
+    JSON.parse(fs.readFileSync('package.json', 'utf-8')).version.should.equal('6.4.0')
+    JSON.parse(fs.readFileSync('package-lock.json', 'utf-8')).version.should.equal('6.4.0')
+  })
+
   it('bumps version # in npm-shrinkwrap.json', async function () {
     mock({
       bump: 'minor',
diff --git a/test/mocks/manifest-6.3.1.json b/test/mocks/manifest-6.3.1.json
new file mode 100644
index 000000000..22a463080
--- /dev/null
+++ b/test/mocks/manifest-6.3.1.json
@@ -0,0 +1,3 @@
+{
+  "version": "6.3.1"
+}
\ No newline at end of file