Skip to content

Commit

Permalink
Fix directives on fragment spread being lost
Browse files Browse the repository at this point in the history
The patch for apollographql#1911 introduced an issue where directives applied to a
fragment spread were lost when the named fragments was reused. This
commit fixes it by ensuring we properly copy those directives.

Fixes apollographql#2124
  • Loading branch information
Sylvain Lebresne committed Sep 2, 2022
1 parent 9057838 commit f81f28f
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 1 deletion.
7 changes: 6 additions & 1 deletion internals-js/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1756,12 +1756,17 @@ class InlineFragmentSelection extends FragmentSelection {
const spread = new FragmentSpreadSelection(this.element().parentType, fragments, candidate.name);
// We use the fragment when the fragments condition is either the same, or a supertype of our current condition.
// If it's the same type, then we don't really want to preserve the current condition, it is included in the
// spread and we can return it directive. But if the fragment condition is a superset, then we should preserve
// spread and we can return it directly. But if the fragment condition is a superset, then we should preserve
// our current condition since it restricts the selection more than the fragment actual does.
if (sameType(typeCondition, candidate.typeCondition)) {
// If we ignore the current condition, then we need to ensure any directive applied to it are preserved.
this.fragmentElement.appliedDirectives.forEach((directive) => {
spread.element().applyDirective(directive.definition!, directive.arguments());
})
return spread;
}
optimizedSelection = selectionSetOf(spread.element().parentType, spread);
break;
}
}
}
Expand Down
102 changes: 102 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3789,4 +3789,106 @@ describe('Named fragments preservation', () => {
}
`);
});

it('preserves directives when fragment not used (because used only once)', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
t: T
}
type T @key(fields : "id") {
id: ID!
a: Int
b: Int
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1);
let operation = operationFromDocument(api, gql`
query test($if: Boolean) {
t {
id
...OnT @include(if: $if)
}
}
fragment OnT on T {
a
b
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "Subgraph1") {
{
t {
id
... on T @include(if: $if) {
a
b
}
}
}
},
}
`);
});

it('preserves directives when fragment is re-used', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
t: T
}
type T @key(fields : "id") {
id: ID!
a: Int
b: Int
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1);
let operation = operationFromDocument(api, gql`
query test($test1: Boolean, $test2: Boolean) {
t {
id
...OnT @include(if: $test1)
...OnT @include(if: $test2)
}
}
fragment OnT on T {
a
b
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Fetch(service: "Subgraph1") {
{
t {
id
...OnT @include(if: $test1)
...OnT @include(if: $test2)
}
}
fragment OnT on T {
a
b
}
},
}
`);
});
});

0 comments on commit f81f28f

Please sign in to comment.