Skip to content

Commit

Permalink
Apply changes from v17 branch
Browse files Browse the repository at this point in the history
Co-authored-by: Yaacov Rydzinski <[email protected]>
  • Loading branch information
JoviDeCroock and yaacovCR committed Sep 8, 2024
1 parent 97b9a8f commit 90412dd
Show file tree
Hide file tree
Showing 33 changed files with 937 additions and 578 deletions.
69 changes: 67 additions & 2 deletions src/execution/__tests__/variables-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ describe('Execute: Handles inputs', () => {

expect(result).to.have.property('errors');
expect(result.errors).to.have.length(1);
expect(result.errors?.[0]?.message).to.match(
expect(result.errors?.[0].message).to.match(
/Argument "value" of required type "String!"/,
);
});
Expand Down Expand Up @@ -1269,7 +1269,7 @@ describe('Execute: Handles inputs', () => {

expect(result).to.have.property('errors');
expect(result.errors).to.have.length(1);
expect(result.errors?.[0]?.message).to.match(
expect(result.errors?.[0].message).to.match(
/Argument "value" of non-null type "String!"/,
);
});
Expand Down Expand Up @@ -1307,6 +1307,22 @@ describe('Execute: Handles inputs', () => {
});
});

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

it('when a nullable argument with a field default is not provided and shadowed by an operation variable', () => {
const result = executeQueryWithFragmentArguments(`
query($x: String = "A") {
Expand Down Expand Up @@ -1411,6 +1427,27 @@ describe('Execute: Handles inputs', () => {
});
});

it('when argument variables with the same name are used directly and recursively', () => {
const result = executeQueryWithFragmentArguments(`
query {
...a(value: "A")
}
fragment a($value: String!) on TestType {
...b(value: "B")
fieldInFragmentA: fieldWithNonNullableStringInput(input: $value)
}
fragment b($value: String!) on TestType {
fieldInFragmentB: fieldWithNonNullableStringInput(input: $value)
}
`);
expect(result).to.deep.equal({
data: {
fieldInFragmentA: '"A"',
fieldInFragmentB: '"B"',
},
});
});

it('when argument passed in as list', () => {
const result = executeQueryWithFragmentArguments(`
query Q($opValue: String = "op") {
Expand All @@ -1433,5 +1470,33 @@ describe('Execute: Handles inputs', () => {
},
});
});

it('when argument passed to a directive', () => {
const result = executeQueryWithFragmentArguments(`
query {
...a(value: true)
}
fragment a($value: Boolean!) on TestType {
fieldWithNonNullableStringInput @skip(if: $value)
}
`);
expect(result).to.deep.equal({
data: {},
});
});

it('when argument passed to a directive on a nested field', () => {
const result = executeQueryWithFragmentArguments(`
query {
...a(value: true)
}
fragment a($value: Boolean!) on TestType {
nested { echo(input: "echo") @skip(if: $value) }
}
`);
expect(result).to.deep.equal({
data: { nested: {} },
});
});
});
});
80 changes: 47 additions & 33 deletions src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,24 @@ import {
} from '../type/directives';
import type { GraphQLSchema } from '../type/schema';

import type { GraphQLVariableSignature } from '../utilities/getVariableSignature';
import { typeFromAST } from '../utilities/typeFromAST';

import { getArgumentValuesFromSpread, getDirectiveValues } from './values';
import { experimentalGetArgumentValues, getDirectiveValues } from './values';

export interface FragmentVariables {
signatures: ObjMap<GraphQLVariableSignature>;
values: ObjMap<unknown>;
}

export interface FieldDetails {
node: FieldNode;
fragmentVariableValues?: ObjMap<unknown> | undefined;
fragmentVariables?: FragmentVariables | undefined;
}

export interface FragmentDetails {
definition: FragmentDefinitionNode;
variableSignatures?: ObjMap<GraphQLVariableSignature> | undefined;
}

/**
Expand All @@ -37,7 +48,7 @@ export interface FieldDetails {
*/
export function collectFields(
schema: GraphQLSchema,
fragments: ObjMap<FragmentDefinitionNode>,
fragments: ObjMap<FragmentDetails>,
variableValues: { [variable: string]: unknown },
runtimeType: GraphQLObjectType,
selectionSet: SelectionSetNode,
Expand Down Expand Up @@ -68,7 +79,7 @@ export function collectFields(
*/
export function collectSubfields(
schema: GraphQLSchema,
fragments: ObjMap<FragmentDefinitionNode>,
fragments: ObjMap<FragmentDetails>,
variableValues: { [variable: string]: unknown },
returnType: GraphQLObjectType,
fieldEntries: ReadonlyArray<FieldDetails>,
Expand All @@ -85,6 +96,7 @@ export function collectSubfields(
entry.node.selectionSet,
subFieldEntries,
visitedFragmentNames,
entry.fragmentVariables,
);
}
}
Expand All @@ -93,41 +105,40 @@ export function collectSubfields(

function collectFieldsImpl(
schema: GraphQLSchema,
fragments: ObjMap<FragmentDefinitionNode>,
fragments: ObjMap<FragmentDetails>,
variableValues: { [variable: string]: unknown },
runtimeType: GraphQLObjectType,
selectionSet: SelectionSetNode,
fields: Map<string, Array<FieldDetails>>,
visitedFragmentNames: Set<string>,
localVariableValues?: { [variable: string]: unknown },
fragmentVariables?: FragmentVariables,
): void {
for (const selection of selectionSet.selections) {
switch (selection.kind) {
case Kind.FIELD: {
const vars = localVariableValues ?? variableValues;
if (!shouldIncludeNode(vars, selection)) {
if (!shouldIncludeNode(selection, variableValues, fragmentVariables)) {
continue;
}
const name = getFieldEntryKey(selection);
const fieldList = fields.get(name);
if (fieldList !== undefined) {
fieldList.push({
node: selection,
fragmentVariableValues: localVariableValues ?? undefined,
fragmentVariables,
});
} else {
fields.set(name, [
{
node: selection,
fragmentVariableValues: localVariableValues ?? undefined,
fragmentVariables,
},
]);
}
break;
}
case Kind.INLINE_FRAGMENT: {
if (
!shouldIncludeNode(variableValues, selection) ||
!shouldIncludeNode(selection, variableValues, fragmentVariables) ||
!doesFragmentConditionMatch(schema, selection, runtimeType)
) {
continue;
Expand All @@ -140,54 +151,50 @@ function collectFieldsImpl(
selection.selectionSet,
fields,
visitedFragmentNames,
fragmentVariables,
);
break;
}
case Kind.FRAGMENT_SPREAD: {
const fragName = selection.name.value;
if (
visitedFragmentNames.has(fragName) ||
!shouldIncludeNode(variableValues, selection)
!shouldIncludeNode(selection, variableValues, fragmentVariables)
) {
continue;
}
visitedFragmentNames.add(fragName);
const fragment = fragments[fragName];
if (
!fragment ||
!doesFragmentConditionMatch(schema, fragment, runtimeType)
!doesFragmentConditionMatch(schema, fragment.definition, runtimeType)
) {
continue;
}

// We need to introduce a concept of shadowing:
//
// - when a fragment defines a variable that is in the parent scope but not given
// in the fragment-spread we need to look at this variable as undefined and check
// whether the definition has a defaultValue, if not remove it from the variableValues.
// - when a fragment does not define a variable we need to copy it over from the parent
// scope as that variable can still get used in spreads later on in the selectionSet.
// - when a value is passed in through the fragment-spread we need to copy over the key-value
// into our variable-values.
const fragmentVariableValues = fragment.variableDefinitions
? getArgumentValuesFromSpread(
const fragmentVariableSignatures = fragment.variableSignatures;
let newFragmentVariables: FragmentVariables | undefined;
if (fragmentVariableSignatures) {
newFragmentVariables = {
signatures: fragmentVariableSignatures,
values: experimentalGetArgumentValues(
selection,
schema,
fragment.variableDefinitions,
Object.values(fragmentVariableSignatures),
variableValues,
localVariableValues,
)
: undefined;
fragmentVariables,
),
};
}

collectFieldsImpl(
schema,
fragments,
variableValues,
runtimeType,
fragment.selectionSet,
fragment.definition.selectionSet,
fields,
visitedFragmentNames,
fragmentVariableValues,
newFragmentVariables,
);
break;
}
Expand All @@ -200,10 +207,16 @@ function collectFieldsImpl(
* directives, where `@skip` has higher precedence than `@include`.
*/
function shouldIncludeNode(
variableValues: { [variable: string]: unknown },
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
variableValues: { [variable: string]: unknown },
fragmentVariables: FragmentVariables | undefined,
): boolean {
const skip = getDirectiveValues(GraphQLSkipDirective, node, variableValues);
const skip = getDirectiveValues(
GraphQLSkipDirective,
node,
variableValues,
fragmentVariables,
);
if (skip?.if === true) {
return false;
}
Expand All @@ -212,6 +225,7 @@ function shouldIncludeNode(
GraphQLIncludeDirective,
node,
variableValues,
fragmentVariables,
);
if (include?.if === false) {
return false;
Expand Down
34 changes: 24 additions & 10 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { invariant } from '../jsutils/invariant';
import { isIterableObject } from '../jsutils/isIterableObject';
import { isObjectLike } from '../jsutils/isObjectLike';
import { isPromise } from '../jsutils/isPromise';
import { mapValue } from '../jsutils/mapValue';
import type { Maybe } from '../jsutils/Maybe';
import { memoize3 } from '../jsutils/memoize3';
import type { ObjMap } from '../jsutils/ObjMap';
Expand All @@ -20,7 +21,6 @@ import { locatedError } from '../error/locatedError';
import type {
DocumentNode,
FieldNode,
FragmentDefinitionNode,
OperationDefinitionNode,
} from '../language/ast';
import { OperationTypeNode } from '../language/ast';
Expand Down Expand Up @@ -52,12 +52,14 @@ import {
import type { GraphQLSchema } from '../type/schema';
import { assertValidSchema } from '../type/validate';

import type { FieldDetails } from './collectFields';
import { getVariableSignature } from '../utilities/getVariableSignature';

import type { FieldDetails, FragmentDetails } from './collectFields';
import {
collectFields,
collectSubfields as _collectSubfields,
} from './collectFields';
import { getArgumentValues, getVariableValues } from './values';
import { experimentalGetArgumentValues, getVariableValues } from './values';

/**
* A memoized collection of relevant subfields with regard to the return
Expand Down Expand Up @@ -107,7 +109,7 @@ const collectSubfields = memoize3(
*/
export interface ExecutionContext {
schema: GraphQLSchema;
fragments: ObjMap<FragmentDefinitionNode>;
fragments: ObjMap<FragmentDetails>;
rootValue: unknown;
contextValue: unknown;
operation: OperationDefinitionNode;
Expand Down Expand Up @@ -290,7 +292,7 @@ export function buildExecutionContext(
} = args;

let operation: OperationDefinitionNode | undefined;
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
const fragments: ObjMap<FragmentDetails> = Object.create(null);
for (const definition of document.definitions) {
switch (definition.kind) {
case Kind.OPERATION_DEFINITION:
Expand All @@ -307,9 +309,18 @@ export function buildExecutionContext(
operation = definition;
}
break;
case Kind.FRAGMENT_DEFINITION:
fragments[definition.name.value] = definition;
case Kind.FRAGMENT_DEFINITION: {
let variableSignatures;
if (definition.variableDefinitions) {
variableSignatures = Object.create(null);
for (const varDef of definition.variableDefinitions) {
const signature = getVariableSignature(schema, varDef);
variableSignatures[signature.name] = signature;
}
}
fragments[definition.name.value] = { definition, variableSignatures };
break;
}
default:
// ignore non-executable definitions
}
Expand Down Expand Up @@ -519,11 +530,11 @@ function executeField(
// Build a JS object of arguments from the field.arguments AST, using the
// variables scope to fulfill any variable references.
// TODO: find a way to memoize, in case this field is within a List type.
const args = getArgumentValues(
const args = experimentalGetArgumentValues(
fieldEntries[0].node,
fieldDef.args,
exeContext.variableValues,
fieldEntries[0].fragmentVariableValues,
fieldEntries[0].fragmentVariables,
);

// The resolve function's optional third argument is a context value that
Expand Down Expand Up @@ -598,7 +609,10 @@ export function buildResolveInfo(
parentType,
path,
schema: exeContext.schema,
fragments: exeContext.fragments,
fragments: mapValue(
exeContext.fragments,
(fragment) => fragment.definition,
),
rootValue: exeContext.rootValue,
operation: exeContext.operation,
variableValues: exeContext.variableValues,
Expand Down
Loading

0 comments on commit 90412dd

Please sign in to comment.