PR Feedback

* Try to make the merge code more approachable for beginners, etc.
* Add test for duplicate plugin values

packages/gatsby/src/bootstrap/index.js

@@ -89,8 +89,9 @@ module.exports = async (args: BootstrapArgs) => {
         return {
           ...themeConfigObj,
           plugins: [
-            { resolve: themeName, options: themeConfig },
             ...(themeConfigObj.plugins || []),
+            // theme plugin is last so it's gatsby-node, etc can override it's declared plugins, like a normal site.
+            { resolve: themeName, options: themeConfig },
           ],
         }
       }

packages/gatsby/src/utils/__tests__/merge-gatsby-config.js

@@ -36,10 +36,40 @@ describe(`Merge gatsby config`, () => {
     })
   })
 
+  it(`Merging plugins uniqs them, keeping the first occurrence`, () => {
+    const basicConfig = {
+      plugins: [`gatsby-mdx`],
+    }
+    const morePlugins = {
+      plugins: [
+        `a-plugin`,
+        `gatsby-mdx`,
+        `b-plugin`,
+        { resolve: `c-plugin`, options: {} },
+      ],
+    }
+    expect(mergeGatsbyConfig(basicConfig, morePlugins)).toEqual({
+      plugins: [
+        `gatsby-mdx`,
+        `a-plugin`,
+        `b-plugin`,
+        { resolve: `c-plugin`, options: {} },
+      ],
+    })
+    expect(mergeGatsbyConfig(morePlugins, basicConfig)).toEqual({
+      plugins: [
+        `a-plugin`,
+        `gatsby-mdx`,
+        `b-plugin`,
+        { resolve: `c-plugin`, options: {} },
+      ],
+    })
+  })
+
   it(`Merging siteMetadata is recursive`, () => {
     const a = {
       siteMetadata: {
-        title: "my site",
+        title: `my site`,
         something: { else: 1 },
       },
     }
@@ -52,7 +82,7 @@ describe(`Merge gatsby config`, () => {
 
     expect(mergeGatsbyConfig(a, b)).toEqual({
       siteMetadata: {
-        title: "my site",
+        title: `my site`,
         something: { else: 1, nested: 2 },
       },
     })
@@ -61,22 +91,22 @@ describe(`Merge gatsby config`, () => {
   it(`Merging proxy is overriden`, () => {
     const a = {
       proxy: {
-        prefix: "/something-not/api",
-        url: "http://examplesite.com/api/",
+        prefix: `/something-not/api`,
+        url: `http://examplesite.com/api/`,
       },
     }
 
     const b = {
       proxy: {
-        prefix: "/api",
-        url: "http://examplesite.com/api/",
+        prefix: `/api`,
+        url: `http://examplesite.com/api/`,
       },
     }
 
     expect(mergeGatsbyConfig(a, b)).toEqual({
       proxy: {
-        prefix: "/api",
-        url: "http://examplesite.com/api/",
+        prefix: `/api`,
+        url: `http://examplesite.com/api/`,
       },
     })
   })

packages/gatsby/src/utils/merge-gatsby-config.js

@@ -1,16 +1,39 @@
-const _ = require("lodash")
+const _ = require(`lodash`)
 /**
  * Defines how a theme object is merged with the user's config
  */
-module.exports = (a, b) =>
-  _.uniq(Object.keys(a).concat(Object.keys(b))).reduce((acc, key) => {
-    const mergeFn = mergeAlgo[key]
-    acc[key] = mergeFn ? mergeFn(a[key], b[key]) : b[key] || a[key]
-    return acc
-  }, {})
+module.exports = (a, b) => {
+  // a and b are gatsby configs, If they have keys, that means there are values to merge
+  const allGatsbyConfigKeysWithAValue = _.uniq(
+    Object.keys(a).concat(Object.keys(b))
+  )
 
-const mergeAlgo = {
-  siteMetadata: (a, b) => _.merge({}, a, b),
-  plugins: (a = [], b = []) => a.concat(b),
-  mapping: (a, b) => _.merge({}, a, b),
+  // reduce the array of mergable keys into a single gatsby config object
+  const mergedConfig = allGatsbyConfigKeysWithAValue.reduce(
+    (config, gatsbyConfigKey) => {
+      // choose a merge function for the config key if there's one defined,
+      // otherwise use the default value merge function
+      const mergeFn = howToMerge[gatsbyConfigKey] || howToMerge.byDefault
+      return {
+        ...config,
+        [gatsbyConfigKey]: mergeFn(a[gatsbyConfigKey], b[gatsbyConfigKey]),
+      }
+    },
+    {}
+  )
+
+  // return the fully merged config
+  return mergedConfig
+}
+const howToMerge = {
+  /**
+   * pick a truthy value by default.
+   * This makes sure that if a single value is defined, that one it used.
+   * We prefer the "right" value, because the user's config will be "on the right"
+   */
+  byDefault: (a, b) => b || a,
+  siteMetadata: (objA, objB) => _.merge({}, objA, objB),
+  // plugins are concatenated and uniq'd, so we don't get two of the same plugin value
+  plugins: (a = [], b = []) => _.uniqWith(a.concat(b), _.isEqual),
+  mapping: (objA, objB) => _.merge({}, objA, objB),
 }