fix(schema): share union type instances (#9052)

gatsbyjs · Oct 19, 2018 · d87881a · d87881a
1 parent b5cb862
commit d87881a
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 42 deletions.
diff --git a/packages/gatsby/src/schema/__tests__/infer-graphql-type-test.js b/packages/gatsby/src/schema/__tests__/infer-graphql-type-test.js
@@ -8,7 +8,8 @@ const path = require(`path`)
 const normalizePath = require(`normalize-path`)
 const { clearTypeExampleValues } = require(`../data-tree-utils`)
 const { typeConflictReporter } = require(`../type-conflict-reporter`)
-const { inferObjectStructureFromNodes } = require(`../infer-graphql-type`)
+const { inferObjectStructureFromNodes, clearUnionTypes } = require(`../infer-graphql-type`)
+const { clearTypeNames } = require(`../create-type-name`)
 
 function queryResult(nodes, fragment, { types = [], ignoreFields } = {}) {
   const schema = new GraphQLSchema({
@@ -698,30 +699,94 @@ describe(`GraphQL type inferance`, () => {
       )
     })
 
-    it(`Creates union types when an array field is linking to multiple node types`, async () => {
-      let result = await queryResult(
-        [{ linked___NODE: [`child_1`, `pet_1`] }],
-        `
-          linked {
-            __typename
-            ... on Child {
-              hair
-            }
-            ... on Pet {
-              species
+    describe(`Creation of union types when array field is linking to multiple types`, () => {
+      beforeEach(() => {
+        clearTypeNames()
+        clearUnionTypes()
+      })
+
+      it(`Creates union types`, async () => {
+        let result = await queryResult(
+          [{ linked___NODE: [`child_1`, `pet_1`] }],
+          `
+            linked {
+              __typename
+              ... on Child {
+                hair
+              }
+              ... on Pet {
+                species
+              }
             }
-          }
-        `,
-        { types }
-      )
-      expect(result.errors).not.toBeDefined()
-      expect(result.data.listNode[0].linked[0].hair).toEqual(`brown`)
-      expect(result.data.listNode[0].linked[0].__typename).toEqual(`Child`)
-      expect(result.data.listNode[0].linked[1].species).toEqual(`dog`)
-      expect(result.data.listNode[0].linked[1].__typename).toEqual(`Pet`)
-      store.dispatch({
-        type: `CREATE_NODE`,
-        payload: { id: `baz`, internal: { type: `Bar` } },
+          `,
+          { types }
+        )
+        expect(result.errors).not.toBeDefined()
+        expect(result.data.listNode[0].linked[0].hair).toEqual(`brown`)
+        expect(result.data.listNode[0].linked[0].__typename).toEqual(`Child`)
+        expect(result.data.listNode[0].linked[1].species).toEqual(`dog`)
+        expect(result.data.listNode[0].linked[1].__typename).toEqual(`Pet`)
+        store.dispatch({
+          type: `CREATE_NODE`,
+          payload: { id: `baz`, internal: { type: `Bar` } },
+        })
+      })
+
+      it(`Uses same union type for same child node types and key`, () => {
+        const fields = inferObjectStructureFromNodes({
+          nodes: [{ test___NODE: [`pet_1`, `child_1`] } ],
+          types,
+        })
+        const fields2 = inferObjectStructureFromNodes({
+          nodes: [{ test___NODE: [`pet_1`, `child_2`] }],
+          types,
+        })
+        expect(fields.test.type).toEqual(fields2.test.type)
+      })
+
+
+      it(`Uses a different type for the same child node types with a different key`, () => {
+        const fields = inferObjectStructureFromNodes({
+          nodes: [{ test___NODE: [`pet_1`, `child_1`] }],
+          types,
+        })
+        const fields2 = inferObjectStructureFromNodes({
+          nodes: [{ differentKey___NODE: [`pet_1`, `child_2`] }],
+          types,
+        })
+        expect(fields.test.type).not.toEqual(fields2.differentKey.type)
+      })
+
+      it(`Uses a different type for different child node types with the same key`, () => {
+        store.dispatch({
+          type: `CREATE_NODE`,
+          payload: { id: `toy_1`, internal: { type: `Toy` } },
+        })
+        const fields = inferObjectStructureFromNodes({
+          nodes: [{ test___NODE: [`pet_1`, `child_1`] } ],
+          types,
+        })
+        const fields2 = inferObjectStructureFromNodes({
+          nodes: [{ test___NODE: [`pet_1`, `child_1`, `toy_1`] }],
+          types: types.concat([{ name: `Toy` }]),
+        })
+        expect(fields.test.type).not.toEqual(fields2.test.type)
+      })
+
+      it(`Creates a new type after schema updates clear union types`, () => {
+        const nodes = [{ test___NODE: [`pet_1`, `child_1`] } ]
+        const fields = inferObjectStructureFromNodes({ nodes, types })
+        clearUnionTypes()
+        const updatedFields = inferObjectStructureFromNodes({ nodes, types })
+        expect(fields.test.type).not.toEqual(updatedFields.test.type)
+      })
+
+      it(`Uses a reliable naming convention`, () => {
+        const nodes = [{ test___NODE: [`pet_1`, `child_1`] } ]
+        inferObjectStructureFromNodes({ nodes, types })
+        clearUnionTypes()
+        const updatedFields = inferObjectStructureFromNodes({ nodes, types })
+        expect(updatedFields.test.type.ofType.name).toEqual(`unionTestNode_2`)
       })
     })
   })

diff --git a/packages/gatsby/src/schema/create-type-name.js b/packages/gatsby/src/schema/create-type-name.js
@@ -1,14 +1,18 @@
 const _ = require(`lodash`)
 
-const seenNames = {}
+const seenNames = new Map()
 
 module.exports = function createTypeName(name) {
   const cameledName = _.camelCase(name)
-  if (seenNames[cameledName]) {
-    seenNames[cameledName] += 1
-    return `${cameledName}_${seenNames[cameledName]}`
+  if (seenNames.has(cameledName)) {
+    seenNames.set(cameledName, seenNames.get(cameledName) + 1)
+    return `${cameledName}_${seenNames.get(cameledName)}`
   } else {
-    seenNames[cameledName] = 1
+    seenNames.set(cameledName, 1)
     return cameledName
   }
 }
+
+module.exports.clearTypeNames = () => {
+  seenNames.clear()
+}
diff --git a/packages/gatsby/src/schema/index.js b/packages/gatsby/src/schema/index.js
@@ -7,8 +7,10 @@ const buildNodeTypes = require(`./build-node-types`)
 const buildNodeConnections = require(`./build-node-connections`)
 const { store } = require(`../redux`)
 const invariant = require(`invariant`)
+const { clearUnionTypes } = require(`./infer-graphql-type`)
 
 module.exports = async ({ parentSpan }) => {
+  clearUnionTypes()
   const typesGQL = await buildNodeTypes({ parentSpan })
   const connections = buildNodeConnections(_.values(typesGQL))
 

diff --git a/packages/gatsby/src/schema/infer-graphql-type.js b/packages/gatsby/src/schema/infer-graphql-type.js
@@ -24,6 +24,7 @@ const {
 const DateType = require(`./types/type-date`)
 const FileType = require(`./types/type-file`)
 const is32BitInteger = require(`../utils/is-32-bit-integer`)
+const unionTypes = new Map()
 
 import type { GraphQLOutputType } from "graphql"
 import type {
@@ -256,21 +257,25 @@ function inferFromFieldName(value, selector, types): GraphQLFieldConfig<*, *> {
     let type
     // If there's more than one type, we'll create a union type.
     if (fields.length > 1) {
-      type = new GraphQLUnionType({
-        name: createTypeName(
-          `Union_${key}_${fields
+      const typeName = `Union_${key}_${fields.map(f => f.name).sort().join(`__`)}`
+
+      if (unionTypes.has(typeName)) {
+        type = unionTypes.get(typeName)
+      }
+
+      if (!type) {
+        type = new GraphQLUnionType({
+          name: createTypeName(`Union_${key}`),
+          description: `Union interface for the field "${key}" for types [${fields
             .map(f => f.name)
             .sort()
-            .join(`__`)}`
-        ),
-        description: `Union interface for the field "${key}" for types [${fields
-          .map(f => f.name)
-          .sort()
-          .join(`, `)}]`,
-        types: fields.map(f => f.nodeObjectType),
-        resolveType: data =>
-          fields.find(f => f.name == data.internal.type).nodeObjectType,
-      })
+            .join(`, `)}]`,
+          types: fields.map(f => f.nodeObjectType),
+          resolveType: data =>
+            fields.find(f => f.name == data.internal.type).nodeObjectType,
+        })
+        unionTypes.set(typeName, type)
+      }
     } else {
       type = fields[0].nodeObjectType
     }
@@ -421,3 +426,7 @@ function _inferObjectStructureFromNodes(
 export function inferObjectStructureFromNodes(options: inferTypeOptions) {
   return _inferObjectStructureFromNodes(options, null)
 }
+
+export function clearUnionTypes() {
+  unionTypes.clear()
+}