Skip to content

Commit

Permalink
fix(gatsby): Fix stack overflow on queries with circular fragments (#…
Browse files Browse the repository at this point in the history
…20936)

* Add failing test

* Fix stack overflow when analysing fragments

* Eslint fixes
  • Loading branch information
vladar authored and GatsbyJS Bot committed Jan 28, 2020
1 parent c09fbf4 commit 094ca4c
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 2 deletions.
65 changes: 65 additions & 0 deletions packages/gatsby/src/query/__tests__/query-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,71 @@ describe(`actual compiling`, () => {
expect(result.get(`mockFile`)).toMatchSnapshot()
})

it(`handles circular fragments`, async () => {
const nodes = [
createGatsbyDoc(
`mockFile`,
`query mockFileQuery {
allDirectory {
nodes {
...Foo
}
}
}
fragment Bar on Directory {
parent {
...Foo
}
}
fragment Foo on Directory {
children {
...Bar
}
}`
),
]

const errors = []
const result = processQueries({
schema,
parsedQueries: nodes,
addError: e => {
errors.push(e)
},
})
expect(errors.length).toEqual(1)
expect(errors[0]).toMatchInlineSnapshot(
{
location: expect.any(Object),
},
`
Object {
"context": Object {
"sourceMessage": "Cannot spread fragment \\"Foo\\" within itself via Bar.
GraphQL request:17:13
16 | children {
17 | ...Bar
| ^
18 | }
GraphQL request:11:13
10 | parent {
11 | ...Foo
| ^
12 | }",
},
"filePath": "mockFile",
"id": "85901",
"location": Any<Object>,
}
`
)
expect(result.size).toEqual(0)
})

it(`removes unused fragments from documents`, async () => {
const nodes = [
createGatsbyDoc(
Expand Down
10 changes: 8 additions & 2 deletions packages/gatsby/src/query/query-compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ const processDefinitions = ({
const determineUsedFragmentsForDefinition = (
definition,
definitionsByName,
fragmentsUsedByFragment
fragmentsUsedByFragment,
visitedFragmentDefinitions = new Set()
) => {
const { def, name, isFragment, filePath } = definition
const cachedUsedFragments = fragmentsUsedByFragment.get(name)
Expand All @@ -443,6 +444,10 @@ const determineUsedFragmentsForDefinition = (
[Kind.FRAGMENT_SPREAD]: node => {
const name = node.name.value
const fragmentDefinition = definitionsByName.get(name)
if (visitedFragmentDefinitions.has(fragmentDefinition)) {
return
}
visitedFragmentDefinitions.add(fragmentDefinition)
if (fragmentDefinition) {
usedFragments.add(name)
const {
Expand All @@ -451,7 +456,8 @@ const determineUsedFragmentsForDefinition = (
} = determineUsedFragmentsForDefinition(
fragmentDefinition,
definitionsByName,
fragmentsUsedByFragment
fragmentsUsedByFragment,
visitedFragmentDefinitions
)
usedFragmentsForFragment.forEach(fragmentName =>
usedFragments.add(fragmentName)
Expand Down

0 comments on commit 094ca4c

Please sign in to comment.