Skip to content

Commit 1ee3f42

Browse files
JoviDeCroockJoviDeCroock
committed
another alternative
1 parent f616e1b commit 1ee3f42

4 files changed

Lines changed: 189 additions & 98 deletions

File tree

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

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ function fieldWithInputArg(
104104
type: GraphQLString,
105105
args: { input: inputArg },
106106
resolve(_, args) {
107-
console.log(args);
108107
if ('input' in args) {
109108
return inspect(args.input);
110109
}
@@ -1199,11 +1198,9 @@ describe('Execute: Handles inputs', () => {
11991198
fieldWithNullableStringInput(input: $value)
12001199
}
12011200
`);
1202-
expect(result).to.deep.equal({
1203-
data: {
1204-
fieldWithNullableStringInput: null,
1205-
},
1206-
});
1201+
1202+
// TODO: Make more exact
1203+
expect(result).to.have.property('errors');
12071204
});
12081205

12091206
it('when the definition has a default and is provided', () => {
@@ -1238,6 +1235,25 @@ describe('Execute: Handles inputs', () => {
12381235
});
12391236
});
12401237

1238+
it('when a definition has a default, is not provided, and spreads another fragment', () => {
1239+
const result = executeQueryWithFragmentArguments(`
1240+
query {
1241+
...a
1242+
}
1243+
fragment a($a: String! = "B") on TestType {
1244+
...b(b: $a)
1245+
}
1246+
fragment b($b: String!) on TestType {
1247+
fieldWithNonNullableStringInput(input: $b)
1248+
}
1249+
`);
1250+
expect(result).to.deep.equal({
1251+
data: {
1252+
fieldWithNonNullableStringInput: '"B"',
1253+
},
1254+
});
1255+
});
1256+
12411257
it('when the definition has a non-nullable default and is provided null', () => {
12421258
const result = executeQueryWithFragmentArguments(`
12431259
query {
@@ -1248,14 +1264,8 @@ describe('Execute: Handles inputs', () => {
12481264
}
12491265
`);
12501266

1251-
// TODO: this seems wrong, our variable definition tells us that `value` is a required string
1252-
// however we pass in null and expect this to be stringified into the input argument of our field.
1253-
// in the spec this sais it should raise a field-error
1254-
expect(result).to.deep.equal({
1255-
data: {
1256-
fieldWithNullableStringInput: 'null',
1257-
},
1258-
});
1267+
// TODO: Make more exact
1268+
expect(result).to.have.property('errors');
12591269
});
12601270

12611271
it('when the definition has no default and is not provided', () => {

‎src/execution/collectFields.ts‎

Lines changed: 18 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import { GraphQLError } from '../error/GraphQLError.js';
21
import { AccumulatorMap } from '../jsutils/AccumulatorMap.js';
3-
import { inspect } from '../jsutils/inspect.js';
42
import { invariant } from '../jsutils/invariant.js';
53
import type { ObjMap } from '../jsutils/ObjMap.js';
64

@@ -11,13 +9,11 @@ import type {
119
InlineFragmentNode,
1210
OperationDefinitionNode,
1311
SelectionSetNode,
14-
ValueNode,
1512
} from '../language/ast.js';
1613
import { OperationTypeNode } from '../language/ast.js';
1714
import { Kind } from '../language/kinds.js';
1815

1916
import type { GraphQLObjectType } from '../type/definition.js';
20-
import { isInputType } from '../type/definition.js';
2117
import { isAbstractType } from '../type/definition.js';
2218
import {
2319
GraphQLDeferDirective,
@@ -27,10 +23,8 @@ import {
2723
import type { GraphQLSchema } from '../type/schema.js';
2824

2925
import { typeFromAST } from '../utilities/typeFromAST.js';
30-
import { valueFromAST } from '../utilities/valueFromAST.js';
31-
import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped.js';
3226

33-
import { getDirectiveValues } from './values.js';
27+
import { getDirectiveValues, getArgumentValuesFromSpread } from './values.js';
3428

3529
export interface DeferUsage {
3630
label: string | undefined;
@@ -224,57 +218,23 @@ function collectFieldsImpl(
224218
// scope as that variable can still get used in spreads later on in the selectionSet.
225219
// - when a value is passed in through the fragment-spread we need to copy over the key-value
226220
// into our variable-values.
227-
if (fragment.variableDefinitions) {
228-
const rawVariables: ObjMap<unknown> = {};
229-
230-
const argumentValueLookup = new Map<string, ValueNode>();
231-
if (selection.arguments) {
232-
for (const argument of selection.arguments) {
233-
argumentValueLookup.set(argument.name.value, argument.value);
234-
}
235-
}
236-
237-
for (const variableDefinition of fragment.variableDefinitions) {
238-
const variableName = variableDefinition.variable.name.value;
239-
const value = argumentValueLookup.get(variableName);
240-
if (value) {
241-
const varType = typeFromAST(context.schema, variableDefinition.type);
242-
if (varType && isInputType(varType)) {
243-
const argumentValue = valueFromAST(value, varType, { ...variableValues, ...fragmentVariableValues });
244-
if (argumentValue !== undefined) {
245-
rawVariables[variableName] = argumentValue
246-
continue;
247-
} else {
248-
throw new GraphQLError(
249-
`Argument "${variableName}" of required type "${inspect(varType)}" ` +
250-
'was not provided.',
251-
{ nodes: selection },
252-
);
253-
}
254-
}
255-
} else if (variableDefinition.defaultValue) {
256-
rawVariables[variableName] = valueFromASTUntyped(variableDefinition.defaultValue, { ...variableValues, ...fragmentVariableValues });
257-
}
258-
}
259-
260-
collectFieldsImpl(
261-
context,
262-
fragment.selectionSet,
263-
groupedFieldSet,
264-
rawVariables,
265-
parentDeferUsage,
266-
newDeferUsage ?? deferUsage,
267-
);
268-
} else {
269-
collectFieldsImpl(
270-
context,
271-
fragment.selectionSet,
272-
groupedFieldSet,
273-
undefined,
274-
parentDeferUsage,
275-
newDeferUsage ?? deferUsage,
276-
);
277-
}
221+
const fragmentArgValues = fragment.variableDefinitions ? getArgumentValuesFromSpread(
222+
selection,
223+
schema,
224+
fragment.variableDefinitions,
225+
variableValues,
226+
fragmentVariableValues,
227+
) : undefined;
228+
229+
collectFieldsImpl(
230+
context,
231+
fragment.selectionSet,
232+
groupedFieldSet,
233+
fragmentArgValues,
234+
parentDeferUsage,
235+
newDeferUsage ?? deferUsage,
236+
);
237+
278238
break;
279239
}
280240
}

‎src/execution/execute.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -624,9 +624,10 @@ function executeField(
624624
// variables scope to fulfill any variable references.
625625
// TODO: find a way to memoize, in case this field is within a List type.
626626
const args = getArgumentValues(
627-
fieldDef,
628627
fieldGroup.fields[0].node,
629-
fieldGroup.fields[0].fragmentVariableValues ?? exeContext.variableValues,
628+
fieldDef.args,
629+
exeContext.variableValues,
630+
fieldGroup.fields[0].fragmentVariableValues
630631
);
631632

632633
// The resolve function's optional third argument is a context value that
@@ -1846,7 +1847,7 @@ function executeSubscription(
18461847

18471848
// Build a JS object of arguments from the field.arguments AST, using the
18481849
// variables scope to fulfill any variable references.
1849-
const args = getArgumentValues(fieldDef, fieldNodes[0], variableValues);
1850+
const args = getArgumentValues(fieldNodes[0], fieldDef.args, variableValues, undefined);
18501851

18511852
// The resolve function's optional third argument is a context value that
18521853
// is provided to every resolve function within an execution. It is commonly

0 commit comments

Comments
 (0)