Skip to content

Commit

Permalink
fixed a regression created by PR apollographql#2967
Browse files Browse the repository at this point in the history
- Previously, directives applied on operations weren't properly attached to their parent elements.
- Changed the `Operation` class to inherit from `DirectiveTargetElement` class, which attaches directives properly.

Related: apollographql#2961
  • Loading branch information
duckki committed Apr 17, 2024
1 parent 8aefa9b commit f8b5119
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 18 deletions.
4 changes: 2 additions & 2 deletions gateway-js/src/resultShaping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export function computeResponse({
}

const parameters = {
schema: operation.schema,
schema: operation.schema(),
variables: {
...operation.collectDefaultedVariableValues(),
// overwrite any defaulted variables if they are provided
Expand All @@ -87,7 +87,7 @@ export function computeResponse({
output: data,
parameters,
path: [],
parentType: operation.schema.schemaDefinition.rootType(operation.rootKind)!,
parentType: operation.schema().schemaDefinition.rootType(operation.rootKind)!,
});

return {
Expand Down
23 changes: 12 additions & 11 deletions internals-js/src/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -879,15 +879,16 @@ function computeFragmentsToKeep(
return toExpand.size === 0 ? fragments : fragments.filter((f) => !toExpand.has(f.name));
}

export class Operation {
export class Operation extends DirectiveTargetElement<Operation> {
constructor(
readonly schema: Schema,
schema: Schema,
readonly rootKind: SchemaRootKind,
readonly selectionSet: SelectionSet,
readonly variableDefinitions: VariableDefinitions,
readonly fragments?: NamedFragments,
readonly name?: string,
readonly directives?: readonly Directive<any>[]) {
directives: readonly Directive<any>[] = []) {
super(schema, directives);
}

// Returns a copy of this operation with the provided updated selection set.
Expand All @@ -898,13 +899,13 @@ export class Operation {
}

return new Operation(
this.schema,
this.schema(),
this.rootKind,
newSelectionSet,
this.variableDefinitions,
this.fragments,
this.name,
this.directives
this.appliedDirectives,
);
}

Expand All @@ -915,13 +916,13 @@ export class Operation {
}

return new Operation(
this.schema,
this.schema(),
this.rootKind,
newSelectionSet,
this.variableDefinitions,
newFragments,
this.name,
this.directives
this.appliedDirectives,
);
}

Expand Down Expand Up @@ -980,13 +981,13 @@ export class Operation {
generateQueryFragments(): Operation {
const [minimizedSelectionSet, fragments] = this.selectionSet.minimizeSelectionSet();
return new Operation(
this.schema,
this.schema(),
this.rootKind,
minimizedSelectionSet,
this.variableDefinitions,
fragments,
this.name,
this.directives
this.appliedDirectives,
);
}

Expand Down Expand Up @@ -1058,7 +1059,7 @@ export class Operation {
}

toString(expandFragments: boolean = false, prettyPrint: boolean = true): string {
return this.selectionSet.toOperationString(this.rootKind, this.variableDefinitions, this.fragments, this.name, this.directives, expandFragments, prettyPrint);
return this.selectionSet.toOperationString(this.rootKind, this.variableDefinitions, this.fragments, this.name, this.appliedDirectives, expandFragments, prettyPrint);
}
}

Expand Down Expand Up @@ -3968,7 +3969,7 @@ export function operationToDocument(operation: Operation): DocumentNode {
name: operation.name ? { kind: Kind.NAME, value: operation.name } : undefined,
selectionSet: operation.selectionSet.toSelectionSetNode(),
variableDefinitions: operation.variableDefinitions.toVariableDefinitionNodes(),
directives: directivesToDirectiveNodes(operation.directives),
directives: directivesToDirectiveNodes(operation.appliedDirectives),
};
const fragmentASTs: DefinitionNode[] = operation.fragments
? operation.fragments?.toFragmentDefinitionNodes()
Expand Down
40 changes: 40 additions & 0 deletions query-planner-js/src/__tests__/buildPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8679,4 +8679,44 @@ describe('handles operations with directives', () => {
}
`);
}); // end of `test`

test('if directives with arguments applied on queries are ok', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
directive @noArgs on QUERY
directive @withArgs(arg1: String) on QUERY
type Query {
test: String!
}
`,
};

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
directive @noArgs on QUERY
directive @withArgs(arg1: String) on QUERY
`,
};

const query = gql`
query @noArgs @withArgs(arg1: "hi") {
test
}
`;

const [api, qp] = composeAndCreatePlanner(subgraph1, subgraph2);
const op = operationFromDocument(api, query);
const queryPlan = qp.buildQueryPlan(op);
const fetch_nodes = findFetchNodes(subgraph1.name, queryPlan.node);
expect(fetch_nodes).toHaveLength(1);
// Note: The query is expected to carry the `@operation` directive.
expect(parse(fetch_nodes[0].operation)).toMatchInlineSnapshot(`
query @noArgs @withArgs(arg1: "hi") {
test
}
`);
});
}); // end of `describe`
10 changes: 5 additions & 5 deletions query-planner-js/src/buildPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3174,7 +3174,7 @@ export class QueryPlanner {
variableDefinitions: operation.variableDefinitions,
fragments: fragments ? new RebasedFragments(fragments) : undefined,
operationName: operation.name,
directives: operation.directives,
directives: operation.appliedDirectives,
assignedDeferLabels,
});

Expand Down Expand Up @@ -3380,13 +3380,13 @@ export class QueryPlanner {
return operation;
}
return new Operation(
operation.schema,
operation.schema(),
operation.rootKind,
updatedSelectionSet,
operation.variableDefinitions,
operation.fragments,
operation.name,
operation.directives,
operation.appliedDirectives,
);
}

Expand Down Expand Up @@ -3535,13 +3535,13 @@ function withoutIntrospection(operation: Operation): Operation {
}

return new Operation(
operation.schema,
operation.schema(),
operation.rootKind,
operation.selectionSet.lazyMap((s) => isIntrospectionSelection(s) ? undefined : s),
operation.variableDefinitions,
operation.fragments,
operation.name,
operation.directives,
operation.appliedDirectives,
);
}

Expand Down

0 comments on commit f8b5119

Please sign in to comment.