From 0bd446f3608ad3e053ce4939c9c7c4110a4ae826 Mon Sep 17 00:00:00 2001 From: Marc-Andre Giroux Date: Wed, 19 Jun 2024 10:36:45 -0400 Subject: [PATCH 1/3] repro bug with repeated fragments spreads and generate query fragments --- internals-js/src/__tests__/operations.test.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internals-js/src/__tests__/operations.test.ts b/internals-js/src/__tests__/operations.test.ts index 7e34d83d5..ceb3c792b 100644 --- a/internals-js/src/__tests__/operations.test.ts +++ b/internals-js/src/__tests__/operations.test.ts @@ -34,6 +34,72 @@ function astSSet(...selections: SelectionNode[]): SelectionSetNode { }; } +describe('generate query fragments', () => { + test('generateQueryFragments handles repeated fragment spreads', () => { + const schema = parseSchema(` + type Query { + entities: [Entity] + } + + union Entity = A | B + + type A { + a: Int + } + + type B { + b: Int + } + `); + + const operation = parseOperation(schema, ` + query { + entities { + ... on B { + ... on B { + ... on B { + ... on B { + b + } + } + } + } + } + } + `); + + const withGeneratedFragments = operation.generateQueryFragments(); + expect(withGeneratedFragments.toString()).toMatchString(` + { + t { + ... on T1 { + a + b + } + ... on T2 { + x + y + } + b + u { + ... on I { + b + } + ... on T1 { + a + b + } + ... on T2 { + x + y + } + } + } + } + `); + }); +}); + describe('fragments optimization', () => { // Takes a query with fragments as inputs, expand all those fragments, and ensures that all the // fragments gets optimized back, and that we get back the exact same query. From ec70fb11e5a40e6b431b56978923bb87ac66222b Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Thu, 20 Jun 2024 14:33:28 -0500 Subject: [PATCH 2/3] Fix nested fragment generation --- internals-js/src/__tests__/operations.test.ts | 40 ++++++++----------- internals-js/src/operations.ts | 7 ++-- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/internals-js/src/__tests__/operations.test.ts b/internals-js/src/__tests__/operations.test.ts index ceb3c792b..6e26b7fc2 100644 --- a/internals-js/src/__tests__/operations.test.ts +++ b/internals-js/src/__tests__/operations.test.ts @@ -69,31 +69,25 @@ describe('generate query fragments', () => { `); const withGeneratedFragments = operation.generateQueryFragments(); + console.log(withGeneratedFragments.toString()); expect(withGeneratedFragments.toString()).toMatchString(` - { - t { - ... on T1 { - a - b - } - ... on T2 { - x - y - } + fragment _generated_onB1_0 on B { + ... on B { b - u { - ... on I { - b - } - ... on T1 { - a - b - } - ... on T2 { - x - y - } - } + } + } + + fragment _generated_onB1_1 on B { + ..._generated_onB1_0 + } + + fragment _generated_onB1_2 on B { + ..._generated_onB1_1 + } + + { + entities { + ..._generated_onB1_2 } } `); diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index 3ac780491..fb2adb6a9 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -1594,16 +1594,17 @@ export class SelectionSet { // No match, so we need to create a new fragment. First, we minimize the // selection set before creating the fragment with it. const [minimizedSelectionSet] = selection.selectionSet.minimizeSelectionSet(namedFragments, seenSelections); + const updatedEquivalentSelectionSetCandidates = seenSelections.get(mockHashCode); // may have changed after previous statement const fragmentDefinition = new NamedFragmentDefinition( this.parentType.schema(), - `_generated_${mockHashCode}_${equivalentSelectionSetCandidates?.length ?? 0}`, + `_generated_${mockHashCode}_${updatedEquivalentSelectionSetCandidates?.length ?? 0}`, selection.element.typeCondition ).setSelectionSet(minimizedSelectionSet); namedFragments.add(fragmentDefinition); // Create a new "hash code" bucket or add to the existing one. - if (equivalentSelectionSetCandidates) { - equivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]); + if (updatedEquivalentSelectionSetCandidates) { + updatedEquivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]); } else { seenSelections.set(mockHashCode, [[selection.selectionSet, fragmentDefinition]]); } From 894de02a14250eab4002d7bd57e72d2f5df8285e Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Thu, 20 Jun 2024 14:41:10 -0500 Subject: [PATCH 3/3] add changeset --- .changeset/calm-candles-rest.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/calm-candles-rest.md diff --git a/.changeset/calm-candles-rest.md b/.changeset/calm-candles-rest.md new file mode 100644 index 000000000..cb2f7525e --- /dev/null +++ b/.changeset/calm-candles-rest.md @@ -0,0 +1,5 @@ +--- +"@apollo/federation-internals": patch +--- + +generateQueryFragments() could generate fragments with naming collisions when nested