Skip to content

Commit

Permalink
add fix for new tests
Browse files Browse the repository at this point in the history
if we allow nesting of defers at the same level then we have to handle the case where a defer is completely empty, not just empty because its fields are also contained within a parent.

this means that collectFIelds must directly return the new defer usages rather than just set the stage for later iteration of the new fields
  • Loading branch information
yaacovCR committed Dec 26, 2023
1 parent 88cc98b commit a25f500
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 43 deletions.
15 changes: 0 additions & 15 deletions src/execution/buildFieldPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export type DeferUsageSet = ReadonlySet<DeferUsage>;
export interface FieldGroup {
fields: ReadonlyArray<FieldDetails>;
deferUsages?: DeferUsageSet | undefined;
knownDeferUsages?: DeferUsageSet | undefined;
}

export type GroupedFieldSet = Map<string, FieldGroup>;
Expand All @@ -21,21 +20,15 @@ export interface NewGroupedFieldSetDetails {
export function buildFieldPlan(
fields: Map<string, ReadonlyArray<FieldDetails>>,
parentDeferUsages: DeferUsageSet = new Set<DeferUsage>(),
knownDeferUsages: DeferUsageSet = new Set<DeferUsage>(),
): {
groupedFieldSet: GroupedFieldSet;
newGroupedFieldSetDetailsMap: Map<DeferUsageSet, NewGroupedFieldSetDetails>;
newDeferUsages: ReadonlyArray<DeferUsage>;
} {
const newDeferUsages: Set<DeferUsage> = new Set<DeferUsage>();
const newKnownDeferUsages = new Set<DeferUsage>(knownDeferUsages);

const groupedFieldSet = new Map<
string,
{
fields: Array<FieldDetails>;
deferUsages: DeferUsageSet;
knownDeferUsages: DeferUsageSet;
}
>();

Expand All @@ -47,7 +40,6 @@ export function buildFieldPlan(
{
fields: Array<FieldDetails>;
deferUsages: DeferUsageSet;
knownDeferUsages: DeferUsageSet;
}
>;
shouldInitiateDefer: boolean;
Expand All @@ -72,10 +64,6 @@ export function buildFieldPlan(
continue;
}
deferUsageSet.add(deferUsage);
if (!knownDeferUsages.has(deferUsage)) {
newDeferUsages.add(deferUsage);
newKnownDeferUsages.add(deferUsage);
}
}
if (inOriginalResult) {
deferUsageSet.clear();
Expand All @@ -99,7 +87,6 @@ export function buildFieldPlan(
fieldGroup = {
fields: [],
deferUsages: deferUsageSet,
knownDeferUsages: newKnownDeferUsages,
};
groupedFieldSet.set(responseKey, fieldGroup);
}
Expand Down Expand Up @@ -140,7 +127,6 @@ export function buildFieldPlan(
fieldGroup = {
fields: [],
deferUsages: deferUsageSet,
knownDeferUsages: newKnownDeferUsages,
};
newGroupedFieldSet.set(responseKey, fieldGroup);
}
Expand All @@ -150,7 +136,6 @@ export function buildFieldPlan(
return {
groupedFieldSet,
newGroupedFieldSetDetailsMap,
newDeferUsages: Array.from(newDeferUsages),
};
}

Expand Down
75 changes: 57 additions & 18 deletions src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ export function collectFields(
variableValues: { [variable: string]: unknown },
runtimeType: GraphQLObjectType,
operation: OperationDefinitionNode,
): Map<string, ReadonlyArray<FieldDetails>> {
): {
fields: Map<string, ReadonlyArray<FieldDetails>>;
newDeferUsages: ReadonlyArray<DeferUsage>;
} {
const groupedFieldSet = new AccumulatorMap<string, FieldDetails>();
const newDeferUsages: Array<DeferUsage> = [];
const context: CollectFieldsContext = {
schema,
fragments,
Expand All @@ -71,8 +75,13 @@ export function collectFields(
visitedFragmentNames: new Set(),
};

collectFieldsImpl(context, operation.selectionSet, groupedFieldSet);
return groupedFieldSet;
collectFieldsImpl(
context,
operation.selectionSet,
groupedFieldSet,
newDeferUsages,
);
return { fields: groupedFieldSet, newDeferUsages };
}

/**
Expand All @@ -93,7 +102,10 @@ export function collectSubfields(
operation: OperationDefinitionNode,
returnType: GraphQLObjectType,
fieldDetails: ReadonlyArray<FieldDetails>,
): Map<string, ReadonlyArray<FieldDetails>> {
): {
fields: Map<string, ReadonlyArray<FieldDetails>>;
newDeferUsages: ReadonlyArray<DeferUsage>;
} {
const context: CollectFieldsContext = {
schema,
fragments,
Expand All @@ -103,6 +115,7 @@ export function collectSubfields(
visitedFragmentNames: new Set(),
};
const subGroupedFieldSet = new AccumulatorMap<string, FieldDetails>();
const newDeferUsages: Array<DeferUsage> = [];

for (const fieldDetail of fieldDetails) {
const node = fieldDetail.node;
Expand All @@ -111,18 +124,23 @@ export function collectSubfields(
context,
node.selectionSet,
subGroupedFieldSet,
newDeferUsages,
fieldDetail.deferUsage,
);
}
}

return subGroupedFieldSet;
return {
fields: subGroupedFieldSet,
newDeferUsages,
};
}

function collectFieldsImpl(
context: CollectFieldsContext,
selectionSet: SelectionSetNode,
groupedFieldSet: AccumulatorMap<string, FieldDetails>,
newDeferUsages: Array<DeferUsage>,
deferUsage?: DeferUsage,
): void {
const {
Expand Down Expand Up @@ -161,12 +179,24 @@ function collectFieldsImpl(
deferUsage,
);

collectFieldsImpl(
context,
selection.selectionSet,
groupedFieldSet,
newDeferUsage ?? deferUsage,
);
if (!newDeferUsage) {
collectFieldsImpl(
context,
selection.selectionSet,
groupedFieldSet,
newDeferUsages,
deferUsage,
);
} else {
newDeferUsages.push(newDeferUsage);
collectFieldsImpl(
context,
selection.selectionSet,
groupedFieldSet,
newDeferUsages,
newDeferUsage,
);
}

break;
}
Expand Down Expand Up @@ -197,14 +227,23 @@ function collectFieldsImpl(
}
if (!newDeferUsage) {
visitedFragmentNames.add(fragName);
collectFieldsImpl(
context,
fragment.selectionSet,
groupedFieldSet,
newDeferUsages,
deferUsage,
);
} else {
newDeferUsages.push(newDeferUsage);
collectFieldsImpl(
context,
fragment.selectionSet,
groupedFieldSet,
newDeferUsages,
newDeferUsage,
);
}

collectFieldsImpl(
context,
fragment.selectionSet,
groupedFieldSet,
newDeferUsage ?? deferUsage,
);
break;
}
}
Expand Down
17 changes: 8 additions & 9 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,18 @@ const buildSubFieldPlan = memoize3(
returnType: GraphQLObjectType,
fieldGroup: FieldGroup,
) => {
const subFields = collectSubfields(
const { fields: subFields, newDeferUsages } = collectSubfields(
exeContext.schema,
exeContext.fragments,
exeContext.variableValues,
exeContext.operation,
returnType,
fieldGroup.fields,
);
return buildFieldPlan(
subFields,
fieldGroup.deferUsages,
fieldGroup.knownDeferUsages,
);
return {
...buildFieldPlan(subFields, fieldGroup.deferUsages),
newDeferUsages,
};
},
);

Expand Down Expand Up @@ -408,14 +407,14 @@ function executeOperation(
);
}

const fields = collectFields(
const { fields, newDeferUsages } = collectFields(
schema,
fragments,
variableValues,
rootType,
operation,
);
const { groupedFieldSet, newGroupedFieldSetDetailsMap, newDeferUsages } =
const { groupedFieldSet, newGroupedFieldSetDetailsMap } =
buildFieldPlan(fields);

const newDeferMap = addNewDeferredFragments(
Expand Down Expand Up @@ -1807,7 +1806,7 @@ function executeSubscription(
);
}

const fields = collectFields(
const { fields } = collectFields(
schema,
fragments,
variableValues,
Expand Down
2 changes: 1 addition & 1 deletion src/validation/rules/SingleFieldSubscriptionsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function SingleFieldSubscriptionsRule(
fragments[definition.name.value] = definition;
}
}
const fields = collectFields(
const { fields } = collectFields(
schema,
fragments,
variableValues,
Expand Down

0 comments on commit a25f500

Please sign in to comment.