Log type conflics when inferring graphql types (gatsbyjs#3905)

* add nodeDescription field to node internal object Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * store extracted example value to not run it 4 times for same nodes (for fields, for input fields, for sort field enums and for group enums) Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * introduce TypeConflictReporter utility class that will store and print type conflicts when creating graphql schema Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * enhance findRootNode function to accept predicate function and rename it to findRootNodeAncestor Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * rewrite extractFieldExamples to be able to keep track of selector path and detect type conflicts early (to not merge values if there is conflict) Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * use type conflict reporter when extracting example value Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * add nested arrays in array of objects to tests Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * rename nodeDescription to description Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * add tests for conflict reporting Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * space out conflict info to be more readable Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * add description for possible conflicts in page context Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * fix schema type conflict in gatsbyjs.org Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * Silence SitePlugin type conflicts Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * just in case clear type examples store in data-tree-utils tests - currently examples are not stored because type is not set - this is just future proofing Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com> * rename getExampleValue to getExampleValues Signed-off-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
slorber · Apr 14, 2018 · 5b1dc1c · 5b1dc1c
1 parent d37502e
commit 5b1dc1c
Show file tree

Hide file tree

Showing 20 changed files with 551 additions and 94 deletions.
diff --git a/packages/gatsby-source-filesystem/src/create-file-node.js b/packages/gatsby-source-filesystem/src/create-file-node.js
@@ -39,13 +39,15 @@ exports.createFileNode = async (pathToFile, pluginOptions = {}) => {
     internal = {
       contentDigest,
       type: `Directory`,
+      description: `Directory "${path.relative(process.cwd(), slashed)}"`,
     }
   } else {
     const contentDigest = await md5File(slashedFile.absolutePath)
     internal = {
       contentDigest,
       mediaType: mime.lookup(slashedFile.ext),
       type: `File`,
+      description: `File "${path.relative(process.cwd(), slashed)}"`,
     }
   }
 

diff --git a/packages/gatsby-source-filesystem/src/create-remote-file-node.js b/packages/gatsby-source-filesystem/src/create-remote-file-node.js
@@ -206,7 +206,7 @@ async function processRemoteNode({ url, store, cache, createNode, auth = {} }) {
 
     // Create the file node.
     const fileNode = await createFileNode(filename, {})
-
+    fileNode.internal.description = `File "${url}"`
     // Override the default plugin as gatsby-source-filesystem needs to
     // be the owner of File nodes or there'll be conflicts if any other
     // File nodes are created through normal usages of

diff --git a/packages/gatsby/src/bootstrap/index.js b/packages/gatsby/src/bootstrap/index.js
@@ -340,6 +340,8 @@ module.exports = async (args: BootstrapArgs) => {
   await require(`../schema`)()
   activity.end()
 
+  require(`../schema/type-conflict-reporter`).printConflicts()
+
   // Extract queries
   activity = report.activityTimer(`extract queries from components`)
   activity.start()

diff --git a/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js b/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js
@@ -156,6 +156,10 @@ exports.onCreatePage = ({ page, boundActionCreators }) => {
         .createHash(`md5`)
         .update(JSON.stringify(page))
         .digest(`hex`),
+      description:
+        page.pluginCreatorId === `Plugin default-site-plugin`
+          ? `Your site's "gatsby-node.js"`
+          : page.pluginCreatorId,
     },
   })
 }

diff --git a/packages/gatsby/src/joi-schemas/joi.js b/packages/gatsby/src/joi-schemas/joi.js
@@ -57,6 +57,7 @@ export const nodeSchema = Joi.object()
       owner: Joi.string().required(),
       fieldOwners: Joi.array(),
       content: Joi.string().allow(``),
+      description: Joi.string(),
     }),
   })
   .unknown()
diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js
@@ -505,6 +505,10 @@ const typeOwners = {}
  * @param {string} node.internal.contentDigest the digest for the content
  * of this node. Helps Gatsby avoid doing extra work on data that hasn't
  * changed.
+ * @param {string} node.internal.description An optional field. Human
+ * readable description of what this node represent / its source. It will
+ * be displayed when type conflicts are found, making it easier to find
+ * and correct type conflicts.
  * @example
  * createNode({
  *   // Data for the node.
@@ -525,6 +529,7 @@ const typeOwners = {}
  *       .digest(`hex`),
  *     mediaType: `text/markdown`, // optional
  *     content: JSON.stringify(fieldData), // optional
+ *     description: `Cool Service: "Title of entry"`, // optional
  *   }
  * })
  */

diff --git a/packages/gatsby/src/schema/__tests__/__snapshots__/data-tree-utils-test.js.snap b/packages/gatsby/src/schema/__tests__/__snapshots__/data-tree-utils-test.js.snap
@@ -35,6 +35,12 @@ Object {
   "name": Object {
     "field": "name",
   },
+  "nestedArrays": Object {
+    "field": "nestedArrays",
+  },
+  "objectsInArray": Object {
+    "field": "objectsInArray",
+  },
 }
 `;
 
@@ -45,21 +51,33 @@ Object {
   ],
   "context": Object {
     "nestedObject": Object {
-      "someOtherProperty": 3,
+      "someOtherProperty": 1,
     },
   },
   "date": "2006-07-22T22:39:53.000Z",
   "emptyArray": null,
   "frontmatter": Object {
-    "blue": 10010,
+    "blue": 100,
     "circle": "happy",
     "date": "2006-07-22T22:39:53.000Z",
     "draft": false,
-    "title": "The world of slash and adventure",
+    "title": "The world of dash and adventure",
   },
-  "hair": 4,
+  "hair": 1,
   "iAmNull": null,
   "key-with..unsupported-values": true,
-  "name": "The Mad Wax",
+  "name": "The Mad Max",
+  "nestedArrays": Array [
+    Array [
+      1,
+    ],
+  ],
+  "objectsInArray": Array [
+    Object {
+      "field1": true,
+      "field2": 1,
+      "field3": "foo",
+    },
+  ],
 }
 `;
diff --git a/packages/gatsby/src/schema/__tests__/data-tree-utils-test.js b/packages/gatsby/src/schema/__tests__/data-tree-utils-test.js
@@ -1,10 +1,19 @@
 const {
-  extractFieldExamples,
+  getExampleValues,
   buildFieldEnumValues,
+  clearTypeExampleValues,
   INVALID_VALUE,
 } = require(`../data-tree-utils`)
+const {
+  typeConflictReporter,
+  TypeConflictEntry,
+} = require(`../type-conflict-reporter`)
 
 describe(`Gatsby data tree utils`, () => {
+  beforeEach(() => {
+    clearTypeExampleValues()
+  })
+
   const nodes = [
     {
       name: `The Mad Max`,
@@ -13,6 +22,8 @@ describe(`Gatsby data tree utils`, () => {
       "key-with..unsupported-values": true,
       emptyArray: [],
       anArray: [1, 2, 3, 4],
+      nestedArrays: [[1, 2, 3], [4, 5, 6]],
+      objectsInArray: [{ field1: true }, { field2: 1 }],
       frontmatter: {
         date: `2006-07-22T22:39:53.000Z`,
         title: `The world of dash and adventure`,
@@ -29,6 +40,8 @@ describe(`Gatsby data tree utils`, () => {
       emptyArray: [undefined, null],
       anArray: [1, 2, 5, 4],
       iAmNull: null,
+      nestedArrays: [[1, 2, 3]],
+      objectsInArray: [{ field3: `foo` }],
       frontmatter: {
         date: `2006-07-22T22:39:53.000Z`,
         title: `The world of slash and adventure`,
@@ -83,32 +96,32 @@ describe(`Gatsby data tree utils`, () => {
   ]
 
   it(`builds field examples from an array of nodes`, () => {
-    expect(extractFieldExamples(nodes)).toMatchSnapshot()
+    expect(getExampleValues(nodes)).toMatchSnapshot()
   })
 
   it(`null fields should have a null value`, () => {
-    expect(extractFieldExamples(nodes).iAmNull).toBeNull()
+    expect(getExampleValues(nodes).iAmNull).toBeNull()
   })
 
   it(`should not mutate the nodes`, () => {
-    extractFieldExamples(nodes)
+    getExampleValues(nodes)
     expect(nodes[0].context.nestedObject).toBeNull()
     expect(nodes[1].context.nestedObject.someOtherProperty).toEqual(1)
     expect(nodes[2].context.nestedObject.someOtherProperty).toEqual(2)
     expect(nodes[3].context.nestedObject.someOtherProperty).toEqual(3)
   })
 
   it(`turns empty or sparse arrays to null`, () => {
-    expect(extractFieldExamples(nodes).emptyArray).toBeNull()
-    expect(extractFieldExamples(nodes).hair).toBeDefined()
+    expect(getExampleValues(nodes).emptyArray).toBeNull()
+    expect(getExampleValues(nodes).hair).toBeDefined()
   })
 
   it(`build enum values for fields from array on nodes`, () => {
     expect(buildFieldEnumValues(nodes)).toMatchSnapshot()
   })
 
   it(`turns polymorphic fields null`, () => {
-    let example = extractFieldExamples([
+    let example = getExampleValues([
       { foo: null },
       { foo: [1] },
       { foo: { field: 1 } },
@@ -117,26 +130,137 @@ describe(`Gatsby data tree utils`, () => {
   })
 
   it(`handles polymorphic arrays`, () => {
-    let example = extractFieldExamples([
+    let example = getExampleValues([
       { foo: [[`foo`, `bar`]] },
       { foo: [{ field: 1 }] },
     ])
     expect(example.foo).toBe(INVALID_VALUE)
   })
 
   it(`doesn't confuse empty fields for polymorhpic ones`, () => {
-    let example = extractFieldExamples([
+    let example = getExampleValues([
       { foo: { bar: 1 } },
       { foo: null },
       { foo: { field: 1 } },
     ])
     expect(example.foo).toEqual({ field: 1, bar: 1 })
 
-    example = extractFieldExamples([
+    example = getExampleValues([
       { foo: [{ bar: 1 }] },
       { foo: null },
       { foo: [{ field: 1 }, { baz: 1 }] },
     ])
     expect(example.foo).toEqual([{ field: 1, bar: 1, baz: 1 }])
   })
 })
+
+describe(`Type conflicts`, () => {
+  let addConflictSpy = jest.spyOn(typeConflictReporter, `addConflict`)
+  let addConflictExampleSpy = jest.spyOn(
+    TypeConflictEntry.prototype,
+    `addExample`
+  )
+
+  beforeEach(() => {
+    clearTypeExampleValues()
+    addConflictExampleSpy.mockReset()
+  })
+
+  afterAll(() => {
+    addConflictSpy.mockRestore()
+    addConflictExampleSpy.mockRestore()
+  })
+
+  it(`Doesn't report conflicts if there are none`, () => {
+    const nodes = [
+      {
+        id: `id1`,
+        string: `string`,
+        number: 5,
+        boolean: true,
+        arrayOfStrings: [`string1`],
+      },
+      {
+        id: `id2`,
+        string: `other string`,
+        number: 3.5,
+        boolean: false,
+        arrayOfStrings: null,
+      },
+    ]
+
+    getExampleValues({ nodes, type: `NoConflict` })
+
+    expect(addConflictExampleSpy).not.toBeCalled()
+  })
+
+  it(`Report type conflicts and its origin`, () => {
+    const nodes = [
+      {
+        id: `id1`,
+        stringOrNumber: `string`,
+        number: 5,
+        boolean: true,
+        arrayOfStrings: [`string1`],
+      },
+      {
+        id: `id2`,
+        stringOrNumber: 5,
+        number: 3.5,
+        boolean: false,
+        arrayOfStrings: null,
+      },
+    ]
+
+    getExampleValues({ nodes, type: `Conflict_1` })
+
+    expect(addConflictSpy).toBeCalled()
+    expect(addConflictSpy).toBeCalledWith(
+      `Conflict_1.stringOrNumber`,
+      expect.any(Array)
+    )
+
+    // expect(addConflictExampleSpy).toBeCalled()
+    expect(addConflictExampleSpy).toHaveBeenCalledTimes(2)
+    expect(addConflictExampleSpy).toBeCalledWith(
+      expect.objectContaining({
+        value: nodes[0].stringOrNumber,
+        type: `string`,
+        parent: nodes[0],
+      })
+    )
+    expect(addConflictExampleSpy).toBeCalledWith(
+      expect.objectContaining({
+        value: nodes[1].stringOrNumber,
+        type: `number`,
+        parent: nodes[1],
+      })
+    )
+  })
+
+  it(`Report conflict when array has mixed types and its origin`, () => {
+    const nodes = [
+      {
+        id: `id1`,
+        arrayOfMixedType: [`string1`, 5, `string2`, true],
+      },
+    ]
+
+    getExampleValues({ nodes, type: `Conflict_2` })
+    expect(addConflictSpy).toBeCalled()
+    expect(addConflictSpy).toBeCalledWith(
+      `Conflict_2.arrayOfMixedType`,
+      expect.any(Array)
+    )
+
+    expect(addConflictExampleSpy).toBeCalled()
+    expect(addConflictExampleSpy).toHaveBeenCalledTimes(1)
+    expect(addConflictExampleSpy).toBeCalledWith(
+      expect.objectContaining({
+        value: nodes[0].arrayOfMixedType,
+        type: `array<boolean|number|string>`,
+        parent: nodes[0],
+      })
+    )
+  })
+})
diff --git a/packages/gatsby/src/schema/__tests__/infer-graphql-input-type-test.js b/packages/gatsby/src/schema/__tests__/infer-graphql-input-type-test.js
@@ -15,6 +15,7 @@ const {
   inferInputObjectStructureFromNodes,
 } = require(`../infer-graphql-input-fields`)
 const createSortField = require(`../create-sort-field`)
+const { clearTypeExampleValues } = require(`../data-tree-utils`)
 
 function queryResult(nodes, query, { types = [] } = {}) {
   const nodeType = new GraphQLObjectType({
@@ -29,7 +30,7 @@ function queryResult(nodes, query, { types = [] } = {}) {
     nodeType,
     connectionFields: () =>
       buildConnectionFields({
-        name: `Test`,
+        name,
         nodes,
         nodeObjectType: nodeType,
       }),
@@ -75,6 +76,10 @@ function queryResult(nodes, query, { types = [] } = {}) {
   return graphql(schema, query)
 }
 
+beforeEach(() => {
+  clearTypeExampleValues()
+})
+
 describe(`GraphQL Input args`, () => {
   const nodes = [
     {

diff --git a/packages/gatsby/src/schema/__tests__/infer-graphql-type-test.js b/packages/gatsby/src/schema/__tests__/infer-graphql-type-test.js
@@ -6,7 +6,7 @@ const {
 } = require(`graphql`)
 const path = require(`path`)
 const normalizePath = require(`normalize-path`)
-
+const { clearTypeExampleValues } = require(`../data-tree-utils`)
 const { inferObjectStructureFromNodes } = require(`../infer-graphql-type`)
 
 function queryResult(nodes, fragment, { types = [] } = {}) {
@@ -48,6 +48,10 @@ function queryResult(nodes, fragment, { types = [] } = {}) {
   )
 }
 
+beforeEach(() => {
+  clearTypeExampleValues()
+})
+
 describe(`GraphQL type inferance`, () => {
   const nodes = [
     {