fix: support variables within input objects (#13)

Author: vladar

Diff for: src/__tests__/regressions/issue-13.test.ts

@@ -0,0 +1,122 @@
+import { buildSchema } from "graphql"
+import { dedent, printQuery, gatsbyApi } from "../test-utils"
+import {
+  buildNodeDefinitions,
+  createSourcingContext,
+  compileNodeQueries,
+  fetchNodeList,
+  fetchNodeById,
+  LimitOffset,
+} from "../.."
+import { IRemoteNode } from "../../types"
+
+// See https://github.com/vladar/gatsby-graphql-toolkit/issues/13
+
+const schema = buildSchema(`
+  type Foo {
+    id: ID!
+    foo: String
+  }
+  input FooInput {
+    id: ID
+    limit: Int
+    offset: Int
+  }
+  type Query {
+    foo(input: FooInput): Foo
+    allFoo(input: FooInput): [Foo]
+  }
+`)
+
+const gatsbyNodeTypes = [
+  {
+    remoteTypeName: `Foo`,
+    queries: `
+      query LIST_FOO {
+        allFoo(input: { limit: $limit offset: $offset }) { ..._FooId_ }
+      }
+      query NODE_FOO {
+        foo(input: { id: $id }) { ..._FooId_ }
+      }
+      fragment _FooId_ on Foo { __typename id }
+    `,
+  },
+]
+
+const documents = compileNodeQueries({
+  schema,
+  gatsbyNodeTypes,
+  customFragments: [],
+})
+
+const fooNode = {
+  remoteTypeName: `Foo`,
+  remoteId: `1`,
+  foo: `fooString`,
+}
+
+it(`adds variable declarations for variables within input objects`, async () => {
+  expect(printQuery(documents, `Foo`)).toEqual(dedent`
+    query LIST_FOO ($limit: Int $offset: Int) {
+      allFoo(input: { limit: $limit, offset: $offset }) {
+        remoteTypeName: __typename
+        ..._FooId_
+      }
+    }
+
+    query NODE_FOO ($id: ID) {
+      foo(input: {id: $id}) {
+        remoteTypeName: __typename
+        ..._FooId_
+      }
+    }
+
+    fragment _FooId_ on Foo {
+      remoteTypeName: __typename
+      remoteId: id
+    }
+  `)
+})
+
+it(`correctly detects field to paginate in list query`, async () => {
+  const context = createSourcingContext({
+    schema,
+    gatsbyNodeDefs: buildNodeDefinitions({ gatsbyNodeTypes, documents }),
+    execute: async () => Promise.resolve({ data: { allFoo: [fooNode] } }),
+    gatsbyApi,
+    gatsbyTypePrefix: `Test`,
+    paginationAdapters: [LimitOffset],
+  })
+
+  const fetchNodes = async () => {
+    const nodes: IRemoteNode[] = []
+    for await (const node of fetchNodeList(context, `Foo`, `LIST_FOO`)) {
+      nodes.push(node)
+    }
+    return nodes
+  }
+
+  // Without the fix it throws:
+  // Cannot find field to paginate in the query LIST_FOO. Make sure you spread IDFragment in your source query:
+  //  query LIST_FOO { field { ...IDFragment } }
+  await expect(fetchNodes()).resolves.toEqual([fooNode])
+})
+
+it(`correctly detects node field in node query`, async () => {
+  const context = createSourcingContext({
+    schema,
+    gatsbyNodeDefs: buildNodeDefinitions({ gatsbyNodeTypes, documents }),
+    execute: async () => Promise.resolve({ data: { foo: fooNode } }),
+    gatsbyApi,
+    gatsbyTypePrefix: `Test`,
+    paginationAdapters: [LimitOffset],
+  })
+
+  // Without the fix it throws:
+  //   Value of the ID field "remoteTypeName" can't be nullish. Got object with keys: foo
+  const fooId = {
+    remoteTypeName: fooNode.remoteTypeName,
+    remoteId: fooNode.remoteId,
+  }
+  await expect(fetchNodeById(context, `Foo`, fooId)).resolves.toEqual(fooNode)
+})

Diff for: src/compile-node-queries/__tests__/compile-node-queries.test.ts

@@ -913,6 +913,39 @@ describe(`Happy path`, () => {
       `)
     })
 
+    it(`supports variables within complex inputs`, () => {
+      const queries = compileNodeQueries({
+        schema,
+        gatsbyNodeTypes: [
+          {
+            remoteTypeName: `Bar`,
+            queries: `
+              query LIST_Bar { allBar(page: { pageNumber: $pageNumber }) { ...BarId } }
+              fragment BarId on Bar { testId }
+            `,
+          },
+        ],
+        customFragments: [`fragment Bar on Bar { createdAt }`],
+      })
+
+      expect(queries.size).toEqual(1)
+      expect(printQuery(queries, `Bar`)).toEqual(dedent`
+        query LIST_Bar($pageNumber: Int) {
+          allBar(page: { pageNumber: $pageNumber }) {
+            remoteTypeName: __typename
+            ...BarId
+            ...Bar
+          }
+        }
+        fragment BarId on Bar {
+          testId
+        }
+        fragment Bar on Bar {
+          createdAt
+        }
+      `)
+    })
+
     it.todo(`Supports deeply nested variables`)
     it.todo(`Supports variables within fragments`)
     it.todo(`Supports deeply nested variables within fragments`)

Diff for: src/compile-node-queries/ast-transformers/add-variable-definitions.ts

@@ -68,10 +68,11 @@ export function addVariableDefinitions({
     FragmentSpread: node => {
       currentDefinition.usedFragments.add(node.name.value)
     },
-    Argument: node => {
+    Variable: node => {
       const inputType = typeInfo.getInputType()
-      if (node.value.kind === "Variable" && inputType) {
-        currentDefinition.variables.set(node.value.name.value, inputType)
+      // FIXME: throw if no inputType found?
+      if (inputType) {
+        currentDefinition.variables.set(node.name.value, inputType)
       }
     },
   }

Diff for: src/source-nodes/utils/field-path-utils.ts

@@ -5,7 +5,6 @@ import {
   FieldNode,
   visit,
   BREAK,
-  VariableNode,
 } from "graphql"
 import * as GraphQLAST from "../../utils/ast-nodes"
 import { IPaginationAdapter } from "../../config/pagination-adapters"
@@ -32,8 +31,12 @@ export function findPaginatedFieldPath(
   const expectedVars = paginationAdapter.expectedVariableNames
 
   if (!expectedVars.length) {
-    // FIXME: use first fragment spread instead of __typename
-    // TODO: consider to always use this instead of isPaginatedField
+    // FIXME: LIST_ queries without variables do not work for Relay connections:
+    //   { myConnection { edges { node { __typename ...IdFragment } } }
+    //   We want to return path to `myConnection` here but it will detect `node` field.
+    //   So maybe allow pagination adapter to decide? i.e. Relay could look into
+    //   type name and return first field with `MyConnection` type
+    // TODO: use first fragment spread instead of __typename
     const hasTypeNameField = (field: FieldNode) =>
       field.selectionSet
         ? field.selectionSet.selections.some(
@@ -44,11 +47,14 @@ export function findPaginatedFieldPath(
   }
 
   const isPaginatedField = (node: FieldNode) => {
-    const variables = (node.arguments ?? [])
-      .map(arg => arg.value)
-      .filter((value): value is VariableNode => value.kind === "Variable")
-      .map(value => value.name.value)
-
+    const variables: Array<string> = []
+    node.arguments?.forEach(arg => {
+      visit(arg, {
+        Variable: variableNode => {
+          variables.push(variableNode.name.value)
+        },
+      })
+    })
     return (
       variables.length > 0 &&
       expectedVars.every(name => variables.includes(name))
@@ -66,7 +72,17 @@ export function findNodeFieldPath(
 ): string[] {
   // For now simply assuming the first field with a variable
   const hasVariableArgument = (node: FieldNode) =>
-    (node.arguments ?? []).some(arg => arg.value.kind === "Variable")
+    (node.arguments ?? []).some(arg => {
+      let hasVariable = false
+      visit(arg, {
+        Variable: () => {
+          hasVariable = true
+          return BREAK
+        },
+      })
+      return hasVariable
+    })
+
   return findFieldPath(document, operationName, hasVariableArgument)
 }