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

fix: cancels deferred fields when overlapping deferred results exhibits null bubbling #3983

Closed
wants to merge 1 commit 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
53 changes: 42 additions & 11 deletions src/execution/IncrementalPublisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,20 @@ export class IncrementalPublisher {
const streams = new Set<StreamRecord>();

const children = this._getChildren(erroringIncrementalDataRecord);
const descendants = this._getDescendants(children);
const descendants = new Set<SubsequentResultRecord>();
this._getDescendants(children, descendants, (child) =>
this._nullsChildSubsequentResultRecord(child, nullPathArray),
);

for (const child of descendants) {
if (!this._nullsChildSubsequentResultRecord(child, nullPathArray)) {
continue;
}
if (isDeferredGroupedFieldSetRecord(erroringIncrementalDataRecord)) {
this.filterSiblings(
nullPathArray,
erroringIncrementalDataRecord,
descendants,
);
}

for (const child of descendants) {
child.filtered = true;

if (isStreamItemsRecord(child)) {
Expand All @@ -371,6 +378,30 @@ export class IncrementalPublisher {
});
}

private filterSiblings(
nullPath: Array<string | number>,
erroringIncrementalDataRecord: DeferredGroupedFieldSetRecord,
descendants: Set<SubsequentResultRecord>,
): Set<SubsequentResultRecord> {
for (const deferredFragmentRecord of erroringIncrementalDataRecord.deferredFragmentRecords) {
for (const deferredGroupedFieldSetRecord of deferredFragmentRecord.deferredGroupedFieldSetRecords) {
if (deferredGroupedFieldSetRecord === erroringIncrementalDataRecord) {
continue;
}
if (this._matchesPath(deferredGroupedFieldSetRecord.path, nullPath)) {
deferredFragmentRecord.deferredGroupedFieldSetRecords.delete(
deferredGroupedFieldSetRecord,
);
deferredFragmentRecord._pending.delete(deferredGroupedFieldSetRecord);

const children = this._getChildren(deferredGroupedFieldSetRecord);
this._getDescendants(children, descendants);
}
}
}
return descendants;
}

private _pendingSourcesToResults(
pendingSources: ReadonlySet<DeferredFragmentRecord | StreamRecord>,
): Array<PendingResult> {
Expand Down Expand Up @@ -694,10 +725,13 @@ export class IncrementalPublisher {
private _getDescendants(
children: ReadonlySet<SubsequentResultRecord>,
descendants = new Set<SubsequentResultRecord>(),
): ReadonlySet<SubsequentResultRecord> {
predicate = (_child: SubsequentResultRecord) => true,
): Set<SubsequentResultRecord> {
for (const child of children) {
descendants.add(child);
this._getDescendants(child.children, descendants);
if (predicate(child)) {
descendants.add(child);
this._getDescendants(child.children, descendants, () => true);
}
}
return descendants;
}
Expand Down Expand Up @@ -760,7 +794,6 @@ export class DeferredGroupedFieldSetRecord {
path: ReadonlyArray<string | number>;
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
groupedFieldSet: GroupedFieldSet;
shouldInitiateDefer: boolean;
errors: Array<GraphQLError>;
data: ObjMap<unknown> | undefined;
sent: boolean;
Expand All @@ -769,12 +802,10 @@ export class DeferredGroupedFieldSetRecord {
path: Path | undefined;
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
groupedFieldSet: GroupedFieldSet;
shouldInitiateDefer: boolean;
}) {
this.path = pathToArray(opts.path);
this.deferredFragmentRecords = opts.deferredFragmentRecords;
this.groupedFieldSet = opts.groupedFieldSet;
this.shouldInitiateDefer = opts.shouldInitiateDefer;
this.errors = [];
this.sent = false;
}
Expand Down
58 changes: 58 additions & 0 deletions src/execution/__tests__/defer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,64 @@ describe('Execute: defer directive', () => {
]);
});

it('Cancels deferred fields when overlapping deferred results exhibits null bubbling', async () => {
const document = parse(`
query {
... @defer {
hero {
nonNullName
id
}
}
... @defer {
hero {
nonNullName
name
}
}
}
`);
const result = await complete(document, {
hero: {
...hero,
nonNullName: Promise.resolve(null),
},
});
expectJSON(result).toDeepEqual([
{
data: {},
pending: [
{ id: '0', path: [] },
{ id: '1', path: [] },
],
hasNext: true,
},
{
incremental: [
{
data: {
hero: null,
},
errors: [
{
message:
'Cannot return null for non-nullable field Hero.nonNullName.',
locations: [
{ line: 5, column: 13 },
{ line: 11, column: 13 },
],
path: ['hero', 'nonNullName'],
},
],
id: '0',
},
],
completed: [{ id: '0' }, { id: '1' }],
hasNext: false,
},
]);
});

it('Deduplicates list fields', async () => {
const document = parse(`
query {
Expand Down
28 changes: 13 additions & 15 deletions src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,10 @@ export interface FieldGroup {

export type GroupedFieldSet = Map<string, FieldGroup>;

export interface GroupedFieldSetDetails {
groupedFieldSet: GroupedFieldSet;
shouldInitiateDefer: boolean;
}

export interface CollectFieldsResult {
groupedFieldSet: GroupedFieldSet;
newGroupedFieldSetDetails: Map<DeferUsageSet, GroupedFieldSetDetails>;
supplementalGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
newDeferredGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
newDeferUsages: ReadonlyArray<DeferUsage>;
}

Expand Down Expand Up @@ -358,7 +354,8 @@ function buildGroupedFieldSets(
parentTargets = NON_DEFERRED_TARGET_SET,
): {
groupedFieldSet: GroupedFieldSet;
newGroupedFieldSetDetails: Map<DeferUsageSet, GroupedFieldSetDetails>;
supplementalGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
newDeferredGroupedFieldSets: Map<DeferUsageSet, GroupedFieldSet>;
} {
const { parentTargetKeys, targetSetDetailsMap } = getTargetSetDetails(
targetsByKey,
Expand All @@ -375,10 +372,11 @@ function buildGroupedFieldSets(
)
: new Map();

const newGroupedFieldSetDetails = new Map<
const supplementalGroupedFieldSets = new Map<
DeferUsageSet,
GroupedFieldSetDetails
GroupedFieldSet
>();
const newDeferredGroupedFieldSets = new Map<DeferUsageSet, GroupedFieldSet>();

for (const [maskingTargets, targetSetDetails] of targetSetDetailsMap) {
const { keys, shouldInitiateDefer } = targetSetDetails;
Expand All @@ -391,16 +389,16 @@ function buildGroupedFieldSets(
);

// All TargetSets that causes new grouped field sets consist only of DeferUsages
// and have shouldInitiateDefer defined
newGroupedFieldSetDetails.set(maskingTargets as DeferUsageSet, {
groupedFieldSet: newGroupedFieldSet,
shouldInitiateDefer,
});
(shouldInitiateDefer
? newDeferredGroupedFieldSets
: supplementalGroupedFieldSets
).set(maskingTargets as DeferUsageSet, newGroupedFieldSet);
}

return {
groupedFieldSet,
newGroupedFieldSetDetails,
supplementalGroupedFieldSets,
newDeferredGroupedFieldSets,
};
}

Expand Down
Loading