Fix id generation for JSON/YAML nodes + more tests (#824)

* Fix id generation for JSON/YAML nodes + more tests

* Debugging

* Remove debug

* Disable devtools when building

Author: KyleAMathews

packages/gatsby-source-filesystem/src/gatsby-node.js

@@ -17,7 +17,10 @@ function readFile(file, pluginOptions, cb) {
       // Stringify date objects.
       const newFile = JSON.parse(
         JSON.stringify({
-          id: `${slashedFile.absolutePath}`,
+          // Don't actually make the File id the absolute path as otherwise
+          // people will use the id for that and ids shouldn't be treated as
+          // useful information.
+          id: `${slashedFile.absolutePath} absPath of file`,
           contentDigest: contentDigest,
           children: [],
           parent: `___SOURCE___`,

packages/gatsby-transformer-json/__tests__/__snapshots__/gatsby-node.js.snap

@@ -22,7 +22,7 @@ Array [
       "content": "{\\"blue\\":false,\\"funny\\":\\"nope\\"}",
       "contentDigest": "f624311d932d73dcd416d2a8bea2b67d",
       "funny": "nope",
-      "id": "f624311d932d73dcd416d2a8bea2b67d",
+      "id": "whatever [1] >>> JSON",
       "mediaType": "application/json",
       "parent": "whatever",
       "type": "Test",
@@ -37,7 +37,7 @@ Array [
     Object {
       "children": Array [
         "foo",
-        "f624311d932d73dcd416d2a8bea2b67d",
+        "whatever [1] >>> JSON",
       ],
       "content": "[{\\"id\\":\\"foo\\",\\"blue\\":true,\\"funny\\":\\"yup\\"},{\\"blue\\":false,\\"funny\\":\\"nope\\"}]",
       "contentDigest": "whatever",

packages/gatsby-transformer-json/__tests__/gatsby-node.js

@@ -1,4 +1,5 @@
 const Promise = require("bluebird")
+const _ = require("lodash")
 
 const { onNodeCreate } = require("../src/gatsby-node")
 
@@ -39,7 +40,7 @@ describe(`Process JSON nodes correctly`, () => {
     })
   })
 
-  it(`If the object has an id, it uses that as the id instead of the contentDigest`, async () => {
+  it(`If the object has an id, it uses that as the id instead of an autogenerated one`, async () => {
     const data = [
       { id: "foo", blue: true, funny: "yup" },
       { blue: false, funny: "nope" },
@@ -58,4 +59,28 @@ describe(`Process JSON nodes correctly`, () => {
       expect(createNode.mock.calls[0][0].id).toEqual("foo")
     })
   })
+
+  it(`the different objects shouldn't get the same ID even if they have the same content`, async () => {
+    const data = [
+      { id: "foo", blue: true, funny: "yup" },
+      { blue: false, funny: "nope" },
+      { blue: false, funny: "nope" },
+      { green: false, funny: "nope" },
+    ]
+    node.content = JSON.stringify(data)
+
+    const createNode = jest.fn()
+    const updateNode = jest.fn()
+    const boundActionCreators = { createNode, updateNode }
+
+    await onNodeCreate({
+      node,
+      loadNodeContent,
+      boundActionCreators,
+    }).then(() => {
+      const ids = createNode.mock.calls.map(object => object[0].id)
+      // Test that they're unique
+      expect(_.uniq(ids).length).toEqual(4)
+    })
+  })
 })

packages/gatsby-transformer-json/src/gatsby-node.js

@@ -21,13 +21,13 @@ async function onNodeCreate({ node, boundActionCreators, loadNodeContent }) {
   }
 
   const content = await loadNodeContent(node)
-  const JSONArray = JSON.parse(content).map(obj => {
+  const JSONArray = JSON.parse(content).map((obj, i) => {
     const objStr = JSON.stringify(obj)
     const contentDigest = crypto.createHash("md5").update(objStr).digest("hex")
 
     return {
       ...obj,
-      id: obj.id ? obj.id : contentDigest,
+      id: obj.id ? obj.id : `${node.id} [${i}] >>> JSON`,
       contentDigest,
       mediaType: `application/json`,
       parent: node.id,

packages/gatsby-transformer-yaml/__tests__/__snapshots__/gatsby-node.js.snap

@@ -9,7 +9,7 @@ Array [
       "content": "{\\"blue\\":true,\\"funny\\":\\"yup\\"}",
       "contentDigest": "73901821b17d5aa9dd6026181f73b64c",
       "funny": "yup",
-      "id": "73901821b17d5aa9dd6026181f73b64c",
+      "id": "whatever [0] >>> YAML",
       "mediaType": "application/json",
       "parent": "whatever",
       "type": "Test",
@@ -22,7 +22,7 @@ Array [
       "content": "{\\"blue\\":false,\\"funny\\":\\"nope\\"}",
       "contentDigest": "f624311d932d73dcd416d2a8bea2b67d",
       "funny": "nope",
-      "id": "f624311d932d73dcd416d2a8bea2b67d",
+      "id": "whatever [1] >>> YAML",
       "mediaType": "application/json",
       "parent": "whatever",
       "type": "Test",
@@ -36,8 +36,8 @@ Array [
   Array [
     Object {
       "children": Array [
-        "73901821b17d5aa9dd6026181f73b64c",
-        "f624311d932d73dcd416d2a8bea2b67d",
+        "whatever [0] >>> YAML",
+        "whatever [1] >>> YAML",
       ],
       "content": "- blue: true
   funny: yup

packages/gatsby-transformer-yaml/__tests__/gatsby-node.js

@@ -1,5 +1,6 @@
 const Promise = require("bluebird")
 const yaml = require("js-yaml")
+const _ = require("lodash")
 
 const { onNodeCreate } = require("../src/gatsby-node")
 
@@ -37,7 +38,7 @@ describe(`Process YAML nodes correctly`, () => {
     })
   })
 
-  it(`If the object has an id, it uses that as the id instead of the contentDigest`, async () => {
+  it(`If the object has an id, it uses that as the id instead of the auto-generated one`, async () => {
     const data = [
       { id: "foo", blue: true, funny: "yup" },
       { blue: false, funny: "nope" },
@@ -56,4 +57,28 @@ describe(`Process YAML nodes correctly`, () => {
       expect(createNode.mock.calls[0][0].id).toEqual("foo")
     })
   })
+
+  it(`the different objects shouldn't get the same ID even if they have the same content`, async () => {
+    const data = [
+      { id: "foo", blue: true, funny: "yup" },
+      { blue: false, funny: "nope" },
+      { blue: false, funny: "nope" },
+      { green: false, funny: "nope" },
+    ]
+    node.content = yaml.safeDump(data)
+
+    const createNode = jest.fn()
+    const updateNode = jest.fn()
+    const boundActionCreators = { createNode, updateNode }
+
+    await onNodeCreate({
+      node,
+      loadNodeContent,
+      boundActionCreators,
+    }).then(() => {
+      const ids = createNode.mock.calls.map(object => object[0].id)
+      // Test that they're unique
+      expect(_.uniq(ids).length).toEqual(4)
+    })
+  })
 })

packages/gatsby-transformer-yaml/src/gatsby-node.js

@@ -12,13 +12,13 @@ async function onNodeCreate({ node, boundActionCreators, loadNodeContent }) {
   }
 
   const content = await loadNodeContent(node)
-  const yamlArray = jsYaml.load(content).map(obj => {
+  const yamlArray = jsYaml.load(content).map((obj, i) => {
     const objStr = JSON.stringify(obj)
     const contentDigest = crypto.createHash("md5").update(objStr).digest("hex")
 
     return {
       ...obj,
-      id: obj.id ? obj.id : contentDigest,
+      id: obj.id ? obj.id : `${node.id} [${i}] >>> YAML`,
       contentDigest,
       type: _.capitalize(node.name),
       mediaType: `application/json`,

packages/gatsby/lib/redux/index.js

@@ -27,7 +27,8 @@ const composeEnhancers = composeWithDevTools({
 })
 
 let store
-if (process.env.NODE_ENV === `test`) {
+// Don't try connecting to devtools server if testing or building.
+if (process.env.NODE_ENV === `test` || process.env.NODE_ENV === `production`) {
   store = Redux.createStore(
     Redux.combineReducers({ ...reducers }),
     initialState