Skip to content

Commit f3b0fa6

Browse files
mjmahonemjmahone
committed
Back to master: quicker to reset and manually redo than to deal with stacked rebase conflicts. Will squash this commit away
1 parent 5060f7a commit f3b0fa6

File tree

12 files changed

+49
-407
lines changed

12 files changed

+49
-407
lines changed

‎src/execution/__tests__/variables-test.ts‎

Lines changed: 0 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,166 +1006,6 @@ describe('Execute: Handles inputs', () => {
10061006
});
10071007
});
10081008

1009-
describe('using fragment arguments', () => {
1010-
it('when there are no fragment arguments', () => {
1011-
const result = executeQuery(`
1012-
query {
1013-
...a
1014-
}
1015-
1016-
fragment a on TestType {
1017-
fieldWithNonNullableStringInput(input: "A")
1018-
}
1019-
`);
1020-
expect(result).to.deep.equal({
1021-
data: {
1022-
fieldWithNonNullableStringInput: '"A"',
1023-
},
1024-
});
1025-
});
1026-
1027-
it('when a value is required and provided', () => {
1028-
const result = executeQuery(`
1029-
query {
1030-
...a(value: "A")
1031-
}
1032-
1033-
fragment a($value: String!) on TestType {
1034-
fieldWithNonNullableStringInput(input: $value)
1035-
}
1036-
`);
1037-
expect(result).to.deep.equal({
1038-
data: {
1039-
fieldWithNonNullableStringInput: '"A"',
1040-
},
1041-
});
1042-
});
1043-
1044-
it('when a value is required and not provided', () => {
1045-
const result = executeQuery(`
1046-
query {
1047-
...a
1048-
}
1049-
1050-
fragment a($value: String!) on TestType {
1051-
fieldWithNullableStringInput(input: $value)
1052-
}
1053-
`);
1054-
expect(result).to.deep.equal({
1055-
data: {
1056-
fieldWithNullableStringInput: null,
1057-
},
1058-
});
1059-
});
1060-
1061-
it('when the definition has a default and is provided', () => {
1062-
const result = executeQuery(`
1063-
query {
1064-
...a(value: "A")
1065-
}
1066-
1067-
fragment a($value: String! = "B") on TestType {
1068-
fieldWithNonNullableStringInput(input: $value)
1069-
}
1070-
`);
1071-
expect(result).to.deep.equal({
1072-
data: {
1073-
fieldWithNonNullableStringInput: '"A"',
1074-
},
1075-
});
1076-
});
1077-
1078-
it('when the definition has a default and is not provided', () => {
1079-
const result = executeQuery(`
1080-
query {
1081-
...a
1082-
}
1083-
1084-
fragment a($value: String! = "B") on TestType {
1085-
fieldWithNonNullableStringInput(input: $value)
1086-
}
1087-
`);
1088-
expect(result).to.deep.equal({
1089-
data: {
1090-
fieldWithNonNullableStringInput: '"B"',
1091-
},
1092-
});
1093-
});
1094-
1095-
it('when the definition has a non-nullable default and is provided null', () => {
1096-
const result = executeQuery(`
1097-
query {
1098-
...a(value: null)
1099-
}
1100-
1101-
fragment a($value: String! = "B") on TestType {
1102-
fieldWithNullableStringInput(input: $value)
1103-
}
1104-
`);
1105-
expect(result).to.deep.equal({
1106-
data: {
1107-
fieldWithNullableStringInput: 'null',
1108-
},
1109-
});
1110-
});
1111-
1112-
it('when the definition has no default and is not provided', () => {
1113-
const result = executeQuery(`
1114-
query {
1115-
...a
1116-
}
1117-
1118-
fragment a($value: String) on TestType {
1119-
fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $value)
1120-
}
1121-
`);
1122-
expect(result).to.deep.equal({
1123-
data: {
1124-
fieldWithNonNullableStringInputAndDefaultArgumentValue:
1125-
'"Hello World"',
1126-
},
1127-
});
1128-
});
1129-
1130-
it('when the argument variable is nested in a complex type', () => {
1131-
const result = executeQuery(`
1132-
query {
1133-
...a(value: "C")
1134-
}
1135-
1136-
fragment a($value: String) on TestType {
1137-
list(input: ["A", "B", $value, "D"])
1138-
}
1139-
`);
1140-
expect(result).to.deep.equal({
1141-
data: {
1142-
list: '["A", "B", "C", "D"]',
1143-
},
1144-
});
1145-
});
1146-
1147-
it('when argument variables are used recursively', () => {
1148-
const result = executeQuery(`
1149-
query {
1150-
...a(aValue: "C")
1151-
}
1152-
1153-
fragment a($aValue: String) on TestType {
1154-
...b(bValue: $aValue)
1155-
}
1156-
1157-
fragment b($bValue: String) on TestType {
1158-
list(input: ["A", "B", $bValue, "D"])
1159-
}
1160-
`);
1161-
expect(result).to.deep.equal({
1162-
data: {
1163-
list: '["A", "B", "C", "D"]',
1164-
},
1165-
});
1166-
});
1167-
});
1168-
11691009
describe('getVariableValues: limit maximum number of coercion errors', () => {
11701010
const doc = parse(`
11711011
query ($input: [String!]) {

‎src/execution/collectFields.ts‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@ import { AccumulatorMap } from '../jsutils/AccumulatorMap';
22
import type { ObjMap } from '../jsutils/ObjMap';
33

44
import type {
5-
ArgumentNode,
65
FieldNode,
76
FragmentDefinitionNode,
87
FragmentSpreadNode,
98
InlineFragmentNode,
109
SelectionSetNode,
11-
ValueNode,
1210
} from '../language/ast';
1311
import { Kind } from '../language/kinds';
1412

@@ -22,8 +20,6 @@ import type { GraphQLSchema } from '../type/schema';
2220

2321
import { typeFromAST } from '../utilities/typeFromAST';
2422

25-
import { visit } from '../language/visitor';
26-
2723
import { getDirectiveValues } from './values';
2824

2925
/**
@@ -143,14 +139,12 @@ function collectFieldsImpl(
143139
) {
144140
continue;
145141
}
146-
const selectionSetWithArgumentsApplied =
147-
selectionSetWithFragmentArgumentsApplied(fragment, selection);
148142
collectFieldsImpl(
149143
schema,
150144
fragments,
151145
variableValues,
152146
runtimeType,
153-
selectionSetWithArgumentsApplied,
147+
fragment.selectionSet,
154148
fields,
155149
visitedFragmentNames,
156150
);
@@ -212,44 +206,3 @@ function doesFragmentConditionMatch(
212206
function getFieldEntryKey(node: FieldNode): string {
213207
return node.alias ? node.alias.value : node.name.value;
214208
}
215-
216-
/**
217-
*
218-
* When a fragment spread is provided with arguments,
219-
* visit that fragment's definition and replace those arguments'
220-
* variable usages with the provided argument value.
221-
*/
222-
function selectionSetWithFragmentArgumentsApplied(
223-
fragment: FragmentDefinitionNode,
224-
fragmentSpread: FragmentSpreadNode,
225-
): SelectionSetNode {
226-
const variableDefinitions = fragment.variableDefinitions;
227-
if (!variableDefinitions) {
228-
return fragment.selectionSet;
229-
}
230-
231-
const providedArguments: Map<string, ArgumentNode> = new Map();
232-
if (fragmentSpread.arguments) {
233-
for (const argument of fragmentSpread.arguments) {
234-
providedArguments.set(argument.name.value, argument);
235-
}
236-
}
237-
238-
const fragmentVariableValues: Map<string, ValueNode> = new Map();
239-
for (const variableDef of variableDefinitions) {
240-
const variableName = variableDef.variable.name.value;
241-
const providedArg = providedArguments.get(variableName);
242-
if (providedArg) {
243-
fragmentVariableValues.set(variableName, providedArg.value);
244-
} else if (variableDef.defaultValue) {
245-
fragmentVariableValues.set(variableName, variableDef.defaultValue);
246-
}
247-
// Otherwise just preserve the variable as-is: it will be treated as unset by the executor.
248-
}
249-
250-
return visit(fragment.selectionSet, {
251-
Variable(node) {
252-
return fragmentVariableValues.get(node.name.value);
253-
},
254-
});
255-
}

‎src/language/__tests__/parser-test.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -591,16 +591,13 @@ describe('Parser', () => {
591591
expect('loc' in result).to.equal(false);
592592
});
593593

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

597-
expect(() => parse(document)).to.not.throw();
598-
});
599-
600-
it('allows parsing fragment spread arguments', () => {
601-
const document = 'fragment a on t { ...b(v: $v) }';
602-
603-
expect(() => parse(document)).to.not.throw();
597+
expect(() =>
598+
parse(document, { allowLegacyFragmentVariables: true }),
599+
).to.not.throw();
600+
expect(() => parse(document)).to.throw('Syntax Error');
604601
});
605602

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

‎src/language/__tests__/printer-test.ts‎

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,10 @@ describe('Printer: Query document', () => {
110110
`);
111111
});
112112

113-
it('prints fragment with variable directives', () => {
113+
it('Legacy: prints fragment with variable directives', () => {
114114
const queryASTWithVariableDirective = parse(
115115
'fragment Foo($foo: TestType @test) on TestType @testDirective { id }',
116+
{ allowLegacyFragmentVariables: true },
116117
);
117118
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
118119
fragment Foo($foo: TestType @test) on TestType @testDirective {
@@ -121,13 +122,14 @@ describe('Printer: Query document', () => {
121122
`);
122123
});
123124

124-
it('correctly prints fragment defined variables', () => {
125+
it('Legacy: correctly prints fragment defined variables', () => {
125126
const fragmentWithVariable = parse(
126127
`
127128
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
128129
id
129130
}
130131
`,
132+
{ allowLegacyFragmentVariables: true },
131133
);
132134
expect(print(fragmentWithVariable)).to.equal(dedent`
133135
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
@@ -136,32 +138,6 @@ describe('Printer: Query document', () => {
136138
`);
137139
});
138140

139-
it('prints fragment spread with arguments', () => {
140-
const queryASTWithVariableDirective = parse(
141-
'fragment Foo on TestType { ...Bar(a: {x: $x}, b: true) }',
142-
);
143-
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
144-
fragment Foo on TestType {
145-
...Bar(a: {x: $x}, b: true)
146-
}
147-
`);
148-
});
149-
150-
it('prints fragment spread with multi-line arguments', () => {
151-
const queryASTWithVariableDirective = parse(
152-
'fragment Foo on TestType { ...Bar(a: {x: $x, y: $y, z: $z, xy: $xy}, b: true, c: "a long string extending arguments over max length") }',
153-
);
154-
expect(print(queryASTWithVariableDirective)).to.equal(dedent`
155-
fragment Foo on TestType {
156-
...Bar(
157-
a: {x: $x, y: $y, z: $z, xy: $xy}
158-
b: true
159-
c: "a long string extending arguments over max length"
160-
)
161-
}
162-
`);
163-
});
164-
165141
it('prints kitchen sink without altering ast', () => {
166142
const ast = parse(kitchenSinkQuery, {
167143
noLocation: true,

‎src/language/__tests__/visitor-test.ts‎

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -455,9 +455,10 @@ describe('Visitor', () => {
455455
]);
456456
});
457457

458-
it('visits arguments defined on fragments', () => {
458+
it('Legacy: visits variables defined in fragments', () => {
459459
const ast = parse('fragment a($v: Boolean = false) on t { f }', {
460460
noLocation: true,
461+
allowLegacyFragmentVariables: true,
461462
});
462463
const visited: Array<any> = [];
463464

@@ -504,49 +505,6 @@ describe('Visitor', () => {
504505
]);
505506
});
506507

507-
it('visits arguments on fragment spreads', () => {
508-
const ast = parse('fragment a on t { ...s(v: false) }', {
509-
noLocation: true,
510-
});
511-
const visited: Array<any> = [];
512-
513-
visit(ast, {
514-
enter(node) {
515-
checkVisitorFnArgs(ast, arguments);
516-
visited.push(['enter', node.kind, getValue(node)]);
517-
},
518-
leave(node) {
519-
checkVisitorFnArgs(ast, arguments);
520-
visited.push(['leave', node.kind, getValue(node)]);
521-
},
522-
});
523-
524-
expect(visited).to.deep.equal([
525-
['enter', 'Document', undefined],
526-
['enter', 'FragmentDefinition', undefined],
527-
['enter', 'Name', 'a'],
528-
['leave', 'Name', 'a'],
529-
['enter', 'NamedType', undefined],
530-
['enter', 'Name', 't'],
531-
['leave', 'Name', 't'],
532-
['leave', 'NamedType', undefined],
533-
['enter', 'SelectionSet', undefined],
534-
['enter', 'FragmentSpread', undefined],
535-
['enter', 'Name', 's'],
536-
['leave', 'Name', 's'],
537-
['enter', 'Argument', { kind: 'BooleanValue', value: false }],
538-
['enter', 'Name', 'v'],
539-
['leave', 'Name', 'v'],
540-
['enter', 'BooleanValue', false],
541-
['leave', 'BooleanValue', false],
542-
['leave', 'Argument', { kind: 'BooleanValue', value: false }],
543-
['leave', 'FragmentSpread', undefined],
544-
['leave', 'SelectionSet', undefined],
545-
['leave', 'FragmentDefinition', undefined],
546-
['leave', 'Document', undefined],
547-
]);
548-
});
549-
550508
it('n', () => {
551509
const ast = parse(kitchenSinkQuery, {
552510
experimentalClientControlledNullability: true,

0 commit comments

Comments
 (0)