fix(gatsby): Fix invalid algorithm of fragment cycles detection (#21128)

gatsbyjs · Feb 1, 2020 · f570db3 · f570db3
1 parent d8dfcc1
commit f570db3
Show file tree

Hide file tree

Showing 2 changed files with 133 additions and 4 deletions.
diff --git a/packages/gatsby/src/query/__tests__/query-compiler.js b/packages/gatsby/src/query/__tests__/query-compiler.js
@@ -276,6 +276,132 @@ describe(`actual compiling`, () => {
     expect(result.get(`mockFile`)).toMatchSnapshot()
   })
 
+  it(`handles multiple fragment usages`, async () => {
+    const nodes = [
+      createGatsbyDoc(
+        `mockFile1`,
+        `query mockFileQuery1 {
+          allDirectory {
+            nodes {
+              ...Foo
+              ...Bar
+            }
+          }
+        }`
+      ),
+      createGatsbyDoc(
+        `mockFile2`,
+        `query mockFileQuery2 {
+          allDirectory {
+            nodes {
+              ...Bar
+            }
+          }
+        }`
+      ),
+      createGatsbyDoc(
+        `mockComponent1`,
+        `fragment Bar on Directory {
+          parent {
+            ...Foo
+          }
+        }
+        fragment Foo on Directory {
+          id
+        }
+        `
+      ),
+    ]
+
+    const errors = []
+    const result = processQueries({
+      schema,
+      parsedQueries: nodes,
+      addError: e => {
+        errors.push(e)
+      },
+    })
+    expect(errors.length).toEqual(0)
+    expect(result.get(`mockFile1`)).toMatchInlineSnapshot(`
+      Object {
+        "hash": "hash",
+        "isHook": false,
+        "isStaticQuery": false,
+        "name": "mockFileQuery1",
+        "originalText": "query mockFileQuery1 {
+                allDirectory {
+                  nodes {
+                    ...Foo
+                    ...Bar
+                  }
+                }
+              }",
+        "path": "mockFile1",
+        "text": "fragment Foo on Directory {
+        id
+      }
+
+      fragment Bar on Directory {
+        id
+        parent {
+          __typename
+          id
+          ...Foo
+        }
+      }
+
+      query mockFileQuery1 {
+        allDirectory {
+          nodes {
+            id
+            ...Foo
+            ...Bar
+          }
+        }
+      }
+      ",
+      }
+    `)
+    expect(result.get(`mockFile2`)).toMatchInlineSnapshot(`
+      Object {
+        "hash": "hash",
+        "isHook": false,
+        "isStaticQuery": false,
+        "name": "mockFileQuery2",
+        "originalText": "query mockFileQuery2 {
+                allDirectory {
+                  nodes {
+                    ...Bar
+                  }
+                }
+              }",
+        "path": "mockFile2",
+        "text": "fragment Bar on Directory {
+        id
+        parent {
+          __typename
+          id
+          ...Foo
+        }
+      }
+
+      fragment Foo on Directory {
+        id
+      }
+
+      query mockFileQuery2 {
+        allDirectory {
+          nodes {
+            id
+            ...Bar
+          }
+        }
+      }
+      ",
+      }
+    `)
+  })
+
   it(`handles circular fragments`, async () => {
     const nodes = [
       createGatsbyDoc(

diff --git a/packages/gatsby/src/query/query-compiler.js b/packages/gatsby/src/query/query-compiler.js
@@ -431,7 +431,7 @@ const determineUsedFragmentsForDefinition = (
   definition,
   definitionsByName,
   fragmentsUsedByFragment,
-  visitedFragments = new Set()
+  traversalPath = []
 ) => {
   const { def, name, isFragment, filePath } = definition
   const cachedUsedFragments = fragmentsUsedByFragment.get(name)
@@ -445,10 +445,12 @@ const determineUsedFragmentsForDefinition = (
         const name = node.name.value
         const fragmentDefinition = definitionsByName.get(name)
         if (fragmentDefinition) {
-          if (visitedFragments.has(name)) {
+          if (traversalPath.includes(name)) {
+            // Already visited this fragment during current traversal.
+            //   Visiting it again will cause a stack overflow
             return
           }
-          visitedFragments.add(name)
+          traversalPath.push(name)
           usedFragments.add(name)
           const {
             usedFragments: usedFragmentsForFragment,
@@ -457,8 +459,9 @@ const determineUsedFragmentsForDefinition = (
             fragmentDefinition,
             definitionsByName,
             fragmentsUsedByFragment,
-            visitedFragments
+            traversalPath
           )
+          traversalPath.pop()
           usedFragmentsForFragment.forEach(fragmentName =>
             usedFragments.add(fragmentName)
           )