refactor(gatsby): Page dependency resolver (#9732)

Anywhere in Gatsby where a graphql resolver returns a node, we need to declare a dependency from the node to the page `context.path`. This code is littered throughout `schema`. Eventually, it would be great to figure out an alternative way to declare these dependencies, since causing side effects in query resolvers isn't ideal, but in the mean time, this PR cleans up the code a bit by introducing a resolver middleware called `pageDependencyResolver`. It executes a given resolver and then creates a page dependency.

Another alternative would be to use [graphql-middleware](https://github.com/prisma/graphql-middleware) to run this on all field resolvers. Perhaps another day.

packages/gatsby/src/redux/actions/add-page-dependency.js

@@ -3,7 +3,7 @@ const _ = require(`lodash`)
 const { store } = require(`../`)
 const { actions } = require(`../actions.js`)
 
-exports.createPageDependency = ({ path, nodeId, connection }) => {
+function createPageDependency({ path, nodeId, connection }) {
   const state = store.getState()
 
   // Check that the dependencies aren't already recorded so we
@@ -36,3 +36,5 @@ exports.createPageDependency = ({ path, nodeId, connection }) => {
   const action = actions.createPageDependency({ path, nodeId, connection })
   store.dispatch(action)
 }
+
+module.exports = createPageDependency

packages/gatsby/src/redux/nodes.js

@@ -84,7 +84,7 @@ exports.loadNodeContent = node => {
  * @returns {Object} node
  */
 exports.getNodeAndSavePathDependency = (id, path) => {
-  const { createPageDependency } = require(`./actions/add-page-dependency`)
+  const createPageDependency = require(`./actions/add-page-dependency`)
   const node = getNode(id)
   createPageDependency({ path, nodeId: id })
   return node

packages/gatsby/src/schema/__tests__/build-node-connections-test.js

@@ -1,17 +1,11 @@
 const { graphql, GraphQLObjectType, GraphQLSchema } = require(`graphql`)
 const _ = require(`lodash`)
-const buildNodeTypes = require(`../build-node-types`)
-const buildNodeConnections = require(`../build-node-connections`)
 
-jest.mock(`../../redux/actions/add-page-dependency`, () => {
-  return {
-    createPageDependency: jest.fn(),
-  }
-})
+const createPageDependency = require(`../../redux/actions/add-page-dependency`)
+jest.mock(`../../redux/actions/add-page-dependency`)
 
-const {
-  createPageDependency,
-} = require(`../../redux/actions/add-page-dependency`)
+const buildNodeTypes = require(`../build-node-types`)
+const buildNodeConnections = require(`../build-node-connections`)
 
 describe(`build-node-connections`, () => {
   let schema, store, types, connections

packages/gatsby/src/schema/__tests__/build-node-types-test.js

@@ -1,17 +1,9 @@
 const { graphql, GraphQLObjectType, GraphQLSchema } = require(`graphql`)
 const _ = require(`lodash`)
+const createPageDependency = require(`../../redux/actions/add-page-dependency`)
+jest.mock(`../../redux/actions/add-page-dependency`)
 const buildNodeTypes = require(`../build-node-types`)
 
-jest.mock(`../../redux/actions/add-page-dependency`, () => {
-  return {
-    createPageDependency: jest.fn(),
-  }
-})
-
-const {
-  createPageDependency,
-} = require(`../../redux/actions/add-page-dependency`)
-
 describe(`build-node-types`, () => {
   let schema, store, types
 
@@ -155,5 +147,9 @@ describe(`build-node-types`, () => {
       path: `foo`,
       nodeId: `p1`,
     })
+    expect(createPageDependency).toHaveBeenCalledWith({
+      path: `foo`,
+      nodeId: `r1`,
+    })
   })
 })

packages/gatsby/src/schema/__tests__/page-dependency-resolver.js

@@ -0,0 +1,10 @@
+const pageDependencyResolver = require(`../page-dependency-resolver`)
+
+describe(`page-dependency-resolver`, () => {
+  it(`should handle nulls in results`, async () => {
+    const innerResolver = () => [null]
+    const resolver = pageDependencyResolver(innerResolver)
+    const result = await resolver({}, {})
+    expect(result).toEqual([null])
+  })
+})

packages/gatsby/src/schema/build-node-connections.js

@@ -10,7 +10,7 @@ const {
 } = require(`./infer-graphql-input-fields-from-fields`)
 const createSortField = require(`./create-sort-field`)
 const buildConnectionFields = require(`./build-connection-fields`)
-const { createPageDependency } = require(`../redux/actions/add-page-dependency`)
+const createPageDependency = require(`../redux/actions/add-page-dependency`)
 const { connectionFromArray } = require(`graphql-skip-limit`)
 const { runQuery } = require(`../db/nodes`)
 

packages/gatsby/src/schema/build-node-types.js

@@ -17,12 +17,8 @@ const {
   inferInputObjectStructureFromNodes,
 } = require(`./infer-graphql-input-fields`)
 const { nodeInterface } = require(`./node-interface`)
-const {
-  getNodes,
-  getNode,
-  getNodeAndSavePathDependency,
-} = require(`../db/nodes`)
-const { createPageDependency } = require(`../redux/actions/add-page-dependency`)
+const { getNodes, getNode } = require(`../db/nodes`)
+const pageDependencyResolver = require(`./page-dependency-resolver`)
 const { setFileNodeRootType } = require(`./types/type-file`)
 const { clearTypeExampleValues } = require(`./data-tree-utils`)
 const { runQuery } = require(`../db/nodes`)
@@ -57,16 +53,12 @@ module.exports = async ({ parentSpan }) => {
       parent: {
         type: nodeInterface,
         description: `The parent of this node.`,
-        resolve(node, a, context) {
-          return getNodeAndSavePathDependency(node.parent, context.path)
-        },
+        resolve: pageDependencyResolver(node => getNode(node.parent)),
       },
       children: {
         type: new GraphQLList(nodeInterface),
         description: `The children of this node.`,
-        resolve(node, a, { path }) {
-          return node.children.map(id => getNodeAndSavePathDependency(id, path))
-        },
+        resolve: pageDependencyResolver(node => node.children.map(getNode)),
       },
     }
 
@@ -90,44 +82,21 @@ module.exports = async ({ parentSpan }) => {
         defaultNodeFields[_.camelCase(`children ${childNodeType}`)] = {
           type: new GraphQLList(processedTypes[childNodeType].nodeObjectType),
           description: `The children of this node of type ${childNodeType}`,
-          resolve(node, a, { path }) {
-            const filteredNodes = node.children
-              .map(id => getNode(id))
-              .filter(
-                ({ internal }) => _.camelCase(internal.type) === childNodeType
-              )
-
-            // Add dependencies for the path
-            filteredNodes.forEach(n =>
-              createPageDependency({
-                path,
-                nodeId: n.id,
-              })
-            )
-            return filteredNodes
-          },
+          resolve: pageDependencyResolver(node =>
+            node.children
+              .map(getNode)
+              .filter(node => _.camelCase(node.internal.type) === childNodeType)
+          ),
         }
       } else {
         defaultNodeFields[_.camelCase(`child ${childNodeType}`)] = {
           type: processedTypes[childNodeType].nodeObjectType,
           description: `The child of this node of type ${childNodeType}`,
-          resolve(node, a, { path }) {
-            const childNode = node.children
-              .map(id => getNode(id))
-              .find(
-                ({ internal }) => _.camelCase(internal.type) === childNodeType
-              )
-
-            if (childNode) {
-              // Add dependencies for the path
-              createPageDependency({
-                path,
-                nodeId: childNode.id,
-              })
-              return childNode
-            }
-            return null
-          },
+          resolve: pageDependencyResolver(node =>
+            node.children
+              .map(getNode)
+              .find(node => _.camelCase(node.internal.type) === childNodeType)
+          ),
         }
       }
     })
@@ -190,8 +159,7 @@ module.exports = async ({ parentSpan }) => {
         name: typeName,
         type: gqlType,
         args: filterFields,
-        async resolve(a, queryArgs, context) {
-          const path = context.path ? context.path : ``
+        resolve: pageDependencyResolver(async (a, queryArgs) => {
           if (!_.isObject(queryArgs)) {
             queryArgs = {}
           }
@@ -207,14 +175,11 @@ module.exports = async ({ parentSpan }) => {
           })
 
           if (results.length > 0) {
-            const result = results[0]
-            const nodeId = result.id
-            createPageDependency({ path, nodeId })
-            return result
+            return results[0]
           } else {
             return null
           }
-        },
+        }),
       },
     }
 

packages/gatsby/src/schema/infer-graphql-type.js

@@ -14,7 +14,7 @@ const { oneLine } = require(`common-tags`)
 
 const { store } = require(`../redux`)
 const { getNode, getNodes, getNodesByType } = require(`../db/nodes`)
-const { createPageDependency } = require(`../redux/actions/add-page-dependency`)
+const pageDependencyResolver = require(`./page-dependency-resolver`)
 const createTypeName = require(`./create-type-name`)
 const createKey = require(`./create-key`)
 const {
@@ -153,30 +153,20 @@ function inferFromMapping(
     return null
   }
 
-  const findNode = (fieldValue, path) => {
-    const linkedNode = _.find(
-      getNodesByType(linkedType),
-      n => _.get(n, linkedField) === fieldValue
-    )
-    if (linkedNode) {
-      createPageDependency({ path, nodeId: linkedNode.id })
-      return linkedNode
-    }
-    return null
-  }
+  const findNode = fieldValue =>
+    getNodesByType(linkedType).find(n => _.get(n, linkedField) === fieldValue)
 
   if (_.isArray(value)) {
     return {
       type: new GraphQLList(matchedTypes[0].nodeObjectType),
-      resolve: (node, a, b, { fieldName }) => {
+      resolve: pageDependencyResolver((node, a, b, { fieldName }) => {
         const fieldValue = node[fieldName]
-
         if (fieldValue) {
-          return fieldValue.map(value => findNode(value, b.path))
+          return fieldValue.map(findNode)
         } else {
           return null
         }
-      },
+      }),
     }
   }
 
@@ -194,21 +184,20 @@ function inferFromMapping(
   }
 }
 
+function findLinkedNodeByField(linkedField, value) {
+  getNodes().find(n => n[linkedField] === value)
+}
+
 export function findLinkedNode(value, linkedField, path) {
   let linkedNode
   // If the field doesn't link to the id, use that for searching.
   if (linkedField) {
-    linkedNode = getNodes().find(n => n[linkedField] === value)
+    linkedNode = findLinkedNodeByField(linkedField, value)
     // Else the field is linking to the node's id, the default.
   } else {
     linkedNode = getNode(value)
   }
-
-  if (linkedNode) {
-    if (path) createPageDependency({ path, nodeId: linkedNode.id })
-    return linkedNode
-  }
-  return null
+  return linkedNode
 }
 
 function inferFromFieldName(value, selector, types): GraphQLFieldConfig<*, *> {
@@ -249,7 +238,7 @@ function inferFromFieldName(value, selector, types): GraphQLFieldConfig<*, *> {
     types.find(type => type.name === node.internal.type)
 
   if (isArray) {
-    const linkedNodes = value.map(v => findLinkedNode(v))
+    const linkedNodes = value.map(getNode)
     linkedNodes.forEach(node => validateLinkedNode(node))
     const fields = linkedNodes.map(node => findNodeType(node))
     fields.forEach((field, i) => validateField(linkedNodes[i], field))
@@ -304,15 +293,9 @@ function inferFromFieldName(value, selector, types): GraphQLFieldConfig<*, *> {
   validateField(linkedNode, field)
   return {
     type: field.nodeObjectType,
-    resolve: (node, a, b = {}) => {
-      let fieldValue = node[key]
-      if (fieldValue) {
-        const result = findLinkedNode(fieldValue, linkedField, b.path)
-        return result
-      } else {
-        return null
-      }
-    },
+    resolve: pageDependencyResolver(node =>
+      findLinkedNode(node[key], linkedField)
+    ),
   }
 }
 

packages/gatsby/src/schema/page-dependency-resolver.js

@@ -0,0 +1,36 @@
+const _ = require(`lodash`)
+const createPageDependency = require(`../redux/actions/add-page-dependency`)
+
+/**
+ * A Graphql resolver middleware that runs `resolver` and creates a
+ * page dependency with the returned node.
+ *
+ * @param resolver A graphql resolver. A function that take arguments
+ * (node, args, context, info) and return a node
+ * @returns A new graphql resolver
+ */
+function pageDependencyResolver(resolver) {
+  return async (node, args, context = {}, info = {}) => {
+    const { path } = context
+    const result = await resolver(node, args, context, info)
+
+    // Call createPageDependency on each result
+    if (path) {
+      const asArray = _.isArray(result) ? result : [result]
+      for (const node of asArray) {
+        if (node) {
+          // using module.exports here so it can be mocked
+          createPageDependency({
+            path,
+            nodeId: node.id,
+          })
+        }
+      }
+    }
+
+    // Finally return the found node
+    return result
+  }
+}
+
+module.exports = pageDependencyResolver