Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Fragment Arguments (a form of parameterized fragments) during execution #3152

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/execution/__tests__/stream-test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assert, expect } from 'chai';
import { assert } from 'chai';
import { describe, it } from 'mocha';

import { expectJSON } from '../../__testUtils__/expectJSON.js';
Expand Down Expand Up @@ -1422,9 +1422,8 @@ describe('Execute: stream directive', () => {
[Symbol.asyncIterator]: () => ({
next: () => {
if (requested) {
/* c8 ignore next 3 */
// Not reached, iterator should end immediately.
expect.fail('Not reached');
// Ignores further errors when filtered.
return Promise.reject(new Error('Oops'));
}
requested = true;
const friend = friends[0];
Expand All @@ -1438,6 +1437,7 @@ describe('Execute: stream directive', () => {
},
return: () => {
returned = true;
// Ignores errors from return.
return Promise.reject(new Error('Oops'));
},
}),
Expand Down
227 changes: 227 additions & 0 deletions src/execution/__tests__/variables-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ function fieldWithInputArg(
};
}

const NestedType: GraphQLObjectType = new GraphQLObjectType({
name: 'NestedType',
fields: {
echo: fieldWithInputArg({ type: GraphQLString }),
},
});

const TestType = new GraphQLObjectType({
name: 'TestType',
fields: {
Expand All @@ -107,6 +114,10 @@ const TestType = new GraphQLObjectType({
defaultValue: 'Hello World',
}),
list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
nested: {
type: NestedType,
resolve: () => ({}),
},
nnList: fieldWithInputArg({
type: new GraphQLNonNull(new GraphQLList(GraphQLString)),
}),
Expand Down Expand Up @@ -1006,6 +1017,222 @@ describe('Execute: Handles inputs', () => {
});
});

describe('using fragment arguments', () => {
it('when there are no fragment arguments', () => {
const result = executeQuery(`
query {
...a
}
fragment a on TestType {
fieldWithNonNullableStringInput(input: "A")
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInput: '"A"',
},
});
});

it('when a value is required and provided', () => {
const result = executeQuery(`
query {
...a(value: "A")
}
fragment a($value: String!) on TestType {
fieldWithNonNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInput: '"A"',
},
});
});

it('when a value is required and not provided', () => {
const result = executeQuery(`
query {
...a
}
fragment a($value: String!) on TestType {
fieldWithNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNullableStringInput: null,
},
});
});

it('when the definition has a default and is provided', () => {
const result = executeQuery(`
query {
...a(value: "A")
}
fragment a($value: String! = "B") on TestType {
fieldWithNonNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInput: '"A"',
},
});
});

it('when the definition has a default and is not provided', () => {
const result = executeQuery(`
query {
...a
}
fragment a($value: String! = "B") on TestType {
fieldWithNonNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInput: '"B"',
},
});
});

it('when the definition has a non-nullable default and is provided null', () => {
const result = executeQuery(`
query {
...a(value: null)
}
fragment a($value: String! = "B") on TestType {
fieldWithNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNullableStringInput: 'null',
},
});
});

it('when the definition has no default and is not provided', () => {
const result = executeQuery(`
query {
...a
}
fragment a($value: String) on TestType {
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInputAndDefaultArgumentValue:
'"Hello World"',
},
});
});

it('when an argument is shadowed by an operation variable', () => {
const result = executeQuery(`
query($x: String! = "A") {
...a(x: "B")
}
fragment a($x: String) on TestType {
fieldWithNullableStringInput(input: $x)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNullableStringInput: '"B"',
},
});
});

it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
const result = executeQuery(`
query($x: String = "A") {
...a
}
fragment a($x: String) on TestType {
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $x)
}
`);
expect(result).to.deep.equal({
data: {
fieldWithNonNullableStringInputAndDefaultArgumentValue:
'"Hello World"',
},
});
});

it('when a fragment is used with different args', () => {
const result = executeQuery(`
query($x: String = "Hello") {
a: nested {
...a(x: "a")
}
b: nested {
...a(x: "b", b: true)
}
hello: nested {
...a(x: $x)
}
}
fragment a($x: String, $b: Boolean = false) on NestedType {
a: echo(input: $x) @skip(if: $b)
b: echo(input: $x) @include(if: $b)
}
`);
expect(result).to.deep.equal({
data: {
a: {
a: '"a"',
},
b: {
b: '"b"',
},
hello: {
a: '"Hello"',
},
},
});
});

it('when the argument variable is nested in a complex type', () => {
const result = executeQuery(`
query {
...a(value: "C")
}
fragment a($value: String) on TestType {
list(input: ["A", "B", $value, "D"])
}
`);
expect(result).to.deep.equal({
data: {
list: '["A", "B", "C", "D"]',
},
});
});

it('when argument variables are used recursively', () => {
const result = executeQuery(`
query {
...a(aValue: "C")
}
fragment a($aValue: String) on TestType {
...b(bValue: $aValue)
}
fragment b($bValue: String) on TestType {
list(input: ["A", "B", $bValue, "D"])
}
`);
expect(result).to.deep.equal({
data: {
list: '["A", "B", "C", "D"]',
},
});
});
});

describe('getVariableValues: limit maximum number of coercion errors', () => {
const doc = parse(`
query ($input: [String!]) {
Expand Down
28 changes: 17 additions & 11 deletions src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
} from '../type/directives.js';
import type { GraphQLSchema } from '../type/schema.js';

import { keyForFragmentSpread } from '../utilities/keyForFragmentSpread.js';
import { substituteFragmentArguments } from '../utilities/substituteFragmentArguments.js';
import { typeFromAST } from '../utilities/typeFromAST.js';

import { getDirectiveValues } from './values.js';
Expand Down Expand Up @@ -124,7 +126,7 @@ function collectFieldsImpl(
selectionSet: SelectionSetNode,
fields: AccumulatorMap<string, FieldNode>,
patches: Array<PatchFields>,
visitedFragmentNames: Set<string>,
visitedFragmentKeys: Set<string>,
): void {
for (const selection of selectionSet.selections) {
switch (selection.kind) {
Expand Down Expand Up @@ -156,7 +158,7 @@ function collectFieldsImpl(
selection.selectionSet,
patchFields,
patches,
visitedFragmentNames,
visitedFragmentKeys,
);
patches.push({
label: defer.label,
Expand All @@ -172,24 +174,24 @@ function collectFieldsImpl(
selection.selectionSet,
fields,
patches,
visitedFragmentNames,
visitedFragmentKeys,
);
}
break;
}
case Kind.FRAGMENT_SPREAD: {
const fragName = selection.name.value;
const fragmentKey = keyForFragmentSpread(selection);

if (!shouldIncludeNode(variableValues, selection)) {
continue;
}

const defer = getDeferValues(operation, variableValues, selection);
if (visitedFragmentNames.has(fragName) && !defer) {
if (visitedFragmentKeys.has(fragmentKey) && !defer) {
continue;
}

const fragment = fragments[fragName];
const fragment = fragments[selection.name.value];
if (
!fragment ||
!doesFragmentConditionMatch(schema, fragment, runtimeType)
Expand All @@ -198,9 +200,13 @@ function collectFieldsImpl(
}

if (!defer) {
visitedFragmentNames.add(fragName);
visitedFragmentKeys.add(fragmentKey);
}

const fragmentSelectionSet = substituteFragmentArguments(
fragment,
selection,
);
if (defer) {
const patchFields = new AccumulatorMap<string, FieldNode>();
collectFieldsImpl(
Expand All @@ -209,10 +215,10 @@ function collectFieldsImpl(
variableValues,
operation,
runtimeType,
fragment.selectionSet,
fragmentSelectionSet,
patchFields,
patches,
visitedFragmentNames,
visitedFragmentKeys,
);
patches.push({
label: defer.label,
Expand All @@ -225,10 +231,10 @@ function collectFieldsImpl(
variableValues,
operation,
runtimeType,
fragment.selectionSet,
fragmentSelectionSet,
fields,
patches,
visitedFragmentNames,
visitedFragmentKeys,
);
}
break;
Expand Down
13 changes: 8 additions & 5 deletions src/language/__tests__/parser-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,13 +607,16 @@ describe('Parser', () => {
expect('loc' in result).to.equal(false);
});

it('Legacy: allows parsing fragment defined variables', () => {
it('allows parsing fragment defined arguments', () => {
const document = 'fragment a($v: Boolean = false) on t { f(v: $v) }';

expect(() =>
parse(document, { allowLegacyFragmentVariables: true }),
).to.not.throw();
expect(() => parse(document)).to.throw('Syntax Error');
expect(() => parse(document)).to.not.throw();
});

it('allows parsing fragment spread arguments', () => {
const document = 'fragment a on t { ...b(v: $v) }';

expect(() => parse(document)).to.not.throw();
});

it('contains location that can be Object.toStringified, JSON.stringified, or jsutils.inspected', () => {
Expand Down
Loading