Skip to content

Commit

Permalink
do not emit pending for empty non-published subsequent results
Browse files Browse the repository at this point in the history
The publish method checks to see if a subsequent result is empty; this same logic should be employed to suppress pending notices for empty records.

This has already been achieved for subsequent results that are children of the initial result, as we generated the pending notices from the list of initially published records.

For subsequent results that are children of other subsequent results, we previously generated the pending notice prior to actually publishing.

This change integrates the logic: the publishing method itself returns a pending notice as required. This results in a bug-fix for subsequent records of other subsequent records as well as a reduction of code for subsequent results to the initial result.
  • Loading branch information
yaacovCR committed Nov 22, 2023
1 parent 0b7590f commit 1d038d6
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 21 deletions.
43 changes: 22 additions & 21 deletions src/execution/IncrementalPublisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,25 +301,20 @@ export class IncrementalPublisher {
initialResultRecord: InitialResultRecord,
data: ObjMap<unknown> | null,
): ExecutionResult | ExperimentalIncrementalExecutionResults {
const pendingSources = new Set<DeferredFragmentRecord | StreamRecord>();
for (const child of initialResultRecord.children) {
if (child.filtered) {
continue;
}
this._publish(child);
const maybePendingSource = this._publish(child);
if (maybePendingSource) {
pendingSources.add(maybePendingSource);
}
}

const errors = initialResultRecord.errors;
const initialResult = errors.length === 0 ? { data } : { errors, data };
const pending = this._pending;
if (pending.size > 0) {
const pendingSources = new Set<DeferredFragmentRecord | StreamRecord>();
for (const subsequentResultRecord of pending) {
const pendingSource = isStreamItemsRecord(subsequentResultRecord)
? subsequentResultRecord.streamRecord
: subsequentResultRecord;
pendingSources.add(pendingSource);
}

if (pendingSources.size > 0) {
return {
initialResult: {
...initialResult,
Expand Down Expand Up @@ -542,13 +537,10 @@ export class IncrementalPublisher {
if (child.filtered) {
continue;
}
const pendingSource = isStreamItemsRecord(child)
? child.streamRecord
: child;
if (!pendingSource.pendingSent) {
newPendingSources.add(pendingSource);
const maybePendingSource = this._publish(child);
if (maybePendingSource) {
newPendingSources.add(maybePendingSource);
}
this._publish(child);
}
if (isStreamItemsRecord(subsequentResultRecord)) {
if (subsequentResultRecord.isFinalRecord) {
Expand Down Expand Up @@ -655,15 +647,18 @@ export class IncrementalPublisher {
return result;
}

private _publish(subsequentResultRecord: SubsequentResultRecord): void {
private _publish(
subsequentResultRecord: SubsequentResultRecord,
): DeferredFragmentRecord | StreamRecord | undefined {
if (isStreamItemsRecord(subsequentResultRecord)) {
if (subsequentResultRecord.isCompleted) {
this._push(subsequentResultRecord);
return;
} else {
this._introduce(subsequentResultRecord);
}

this._introduce(subsequentResultRecord);
return;
const stream = subsequentResultRecord.streamRecord;
return stream.pendingSent ? undefined : stream;
}

if (subsequentResultRecord._pending.size > 0) {
Expand All @@ -673,7 +668,13 @@ export class IncrementalPublisher {
subsequentResultRecord.children.size > 0
) {
this._push(subsequentResultRecord);
} else {
return;
}

return subsequentResultRecord.pendingSent
? undefined
: subsequentResultRecord;
}

private _getChildren(
Expand Down
54 changes: 54 additions & 0 deletions src/execution/__tests__/defer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const anotherNestedObject = new GraphQLObjectType({

const hero = {
name: 'Luke',
lastName: 'SkyWalker',
id: 1,
friends,
nestedObject,
Expand Down Expand Up @@ -112,6 +113,7 @@ const heroType = new GraphQLObjectType({
fields: {
id: { type: GraphQLID },
name: { type: GraphQLString },
lastName: { type: GraphQLString },
nonNullName: { type: new GraphQLNonNull(GraphQLString) },
friends: {
type: new GraphQLList(friendType),
Expand Down Expand Up @@ -566,6 +568,58 @@ describe('Execute: defer directive', () => {
]);
});

it('Separately emits defer fragments with different labels with varying subfields with superimposed masked defer', async () => {
const document = parse(`
query HeroNameQuery {
... @defer(label: "DeferID") {
hero {
id
}
}
... @defer(label: "DeferName") {
hero {
name
lastName
... @defer {
lastName
}
}
}
}
`);
const result = await complete(document);
expectJSON(result).toDeepEqual([
{
data: {},
pending: [
{ id: '0', path: [], label: 'DeferID' },
{ id: '1', path: [], label: 'DeferName' },
],
hasNext: true,
},
{
incremental: [
{
data: { hero: {} },
id: '0',
},
{
data: { id: '1' },
id: '0',
subPath: ['hero'],
},
{
data: { name: 'Luke', lastName: 'SkyWalker' },
id: '1',
subPath: ['hero'],
},
],
completed: [{ id: '0' }, { id: '1' }],
hasNext: false,
},
]);
});

it('Separately emits defer fragments with different labels with varying subfields that return promises', async () => {
const document = parse(`
query HeroNameQuery {
Expand Down

0 comments on commit 1d038d6

Please sign in to comment.