Skip to content

fix: suppress records for deferred fragments that are completely empty #3984

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 7, 2023
Merged

fix: suppress records for deferred fragments that are completely empty #3984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions src/execution/IncrementalPublisher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,10 +666,13 @@ export class IncrementalPublisher {
return;
}

if (subsequentResultRecord._pending.size === 0) {
this._push(subsequentResultRecord);
} else {
if (subsequentResultRecord._pending.size > 0) {
this._introduce(subsequentResultRecord);
} else if (
subsequentResultRecord.deferredGroupedFieldSetRecords.size > 0 ||
subsequentResultRecord.children.size > 0
) {
this._push(subsequentResultRecord);
}
}

Expand Down
172 changes: 39 additions & 133 deletions src/execution/__tests__/defer-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,21 +394,13 @@ describe('Execute: defer directive', () => {
}
`);
const result = await complete(document);
expectJSON(result).toDeepEqual([
{
data: {
hero: {
name: 'Luke',
},
expectJSON(result).toDeepEqual({
data: {
hero: {
name: 'Luke',
},
pending: [{ id: '0', path: ['hero'], label: 'DeferTop' }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
});
});
it('Can defer a fragment that is also not deferred, non-deferred fragment is first', async () => {
const document = parse(`
Expand All @@ -423,21 +415,13 @@ describe('Execute: defer directive', () => {
}
`);
const result = await complete(document);
expectJSON(result).toDeepEqual([
{
data: {
hero: {
name: 'Luke',
},
expectJSON(result).toDeepEqual({
data: {
hero: {
name: 'Luke',
},
pending: [{ id: '0', path: ['hero'], label: 'DeferTop' }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
});
});

it('Can defer an inline fragment', async () => {
Expand Down Expand Up @@ -481,19 +465,11 @@ describe('Execute: defer directive', () => {
}
`);
const result = await complete(document);
expectJSON(result).toDeepEqual([
{
data: {
hero: {},
},
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
expectJSON(result).toDeepEqual({
data: {
hero: {},
},
]);
});
});

it('Can separately emit defer fragments with different labels with varying fields', async () => {
Expand Down Expand Up @@ -775,40 +751,18 @@ describe('Execute: defer directive', () => {
data: { hero: { friends: [{}, {}, {}] } },
pending: [
{ id: '0', path: ['hero', 'friends', 0] },
{ id: '1', path: ['hero', 'friends', 0] },
{ id: '2', path: ['hero', 'friends', 0] },
{ id: '3', path: ['hero', 'friends', 0] },
{ id: '4', path: ['hero', 'friends', 1] },
{ id: '5', path: ['hero', 'friends', 1] },
{ id: '6', path: ['hero', 'friends', 1] },
{ id: '7', path: ['hero', 'friends', 1] },
{ id: '8', path: ['hero', 'friends', 2] },
{ id: '9', path: ['hero', 'friends', 2] },
{ id: '10', path: ['hero', 'friends', 2] },
{ id: '11', path: ['hero', 'friends', 2] },
{ id: '1', path: ['hero', 'friends', 1] },
{ id: '2', path: ['hero', 'friends', 2] },
],
hasNext: true,
},
{
incremental: [
{ data: { id: '2', name: 'Han' }, id: '0' },
{ data: { id: '3', name: 'Leia' }, id: '4' },
{ data: { id: '4', name: 'C-3PO' }, id: '8' },
],
completed: [
{ id: '1' },
{ id: '2' },
{ id: '3' },
{ id: '5' },
{ id: '6' },
{ id: '7' },
{ id: '9' },
{ id: '10' },
{ id: '11' },
{ id: '0' },
{ id: '4' },
{ id: '8' },
{ data: { id: '3', name: 'Leia' }, id: '1' },
{ data: { id: '4', name: 'C-3PO' }, id: '2' },
],
completed: [{ id: '0' }, { id: '1' }, { id: '2' }],
hasNext: false,
},
]);
Expand Down Expand Up @@ -1494,21 +1448,13 @@ describe('Execute: defer directive', () => {
}
`);
const result = await complete(document);
expectJSON(result).toDeepEqual([
{
data: {
hero: {
friends: [{ name: 'Han' }, { name: 'Leia' }, { name: 'C-3PO' }],
},
expectJSON(result).toDeepEqual({
data: {
hero: {
friends: [{ name: 'Han' }, { name: 'Leia' }, { name: 'C-3PO' }],
},
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
});
});

it('Deduplicates async iterable list fields', async () => {
Expand All @@ -1534,17 +1480,9 @@ describe('Execute: defer directive', () => {
},
},
});
expectJSON(result).toDeepEqual([
{
data: { hero: { friends: [{ name: 'Han' }] } },
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
expectJSON(result).toDeepEqual({
data: { hero: { friends: [{ name: 'Han' }] } },
});
});

it('Deduplicates empty async iterable list fields', async () => {
Expand All @@ -1571,17 +1509,9 @@ describe('Execute: defer directive', () => {
},
},
});
expectJSON(result).toDeepEqual([
{
data: { hero: { friends: [] } },
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
expectJSON(result).toDeepEqual({
data: { hero: { friends: [] } },
});
});

it('Does not deduplicate list fields with non-overlapping fields', async () => {
Expand Down Expand Up @@ -1655,17 +1585,9 @@ describe('Execute: defer directive', () => {
friends: () => [],
},
});
expectJSON(result).toDeepEqual([
{
data: { hero: { friends: [] } },
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
expectJSON(result).toDeepEqual({
data: { hero: { friends: [] } },
});
});

it('Deduplicates null object fields', async () => {
Expand All @@ -1689,17 +1611,9 @@ describe('Execute: defer directive', () => {
nestedObject: () => null,
},
});
expectJSON(result).toDeepEqual([
{
data: { hero: { nestedObject: null } },
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
expectJSON(result).toDeepEqual({
data: { hero: { nestedObject: null } },
});
});

it('Deduplicates promise object fields', async () => {
Expand All @@ -1722,17 +1636,9 @@ describe('Execute: defer directive', () => {
nestedObject: () => Promise.resolve({ name: 'foo' }),
},
});
expectJSON(result).toDeepEqual([
{
data: { hero: { nestedObject: { name: 'foo' } } },
pending: [{ id: '0', path: ['hero'] }],
hasNext: true,
},
{
completed: [{ id: '0' }],
hasNext: false,
},
]);
expectJSON(result).toDeepEqual({
data: { hero: { nestedObject: { name: 'foo' } } },
});
});

it('Handles errors thrown in deferred fragments', async () => {
Expand Down
12 changes: 4 additions & 8 deletions src/execution/__tests__/stream-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,33 +1780,29 @@ describe('Execute: stream directive', () => {
nestedFriendList: [],
},
},
pending: [
{ id: '0', path: ['nestedObject'] },
{ id: '1', path: ['nestedObject', 'nestedFriendList'] },
],
pending: [{ id: '0', path: ['nestedObject', 'nestedFriendList'] }],
hasNext: true,
},
{
incremental: [
{
items: [{ id: '1', name: 'Luke' }],
id: '1',
id: '0',
},
],
completed: [{ id: '0' }],
hasNext: true,
},
{
incremental: [
{
items: [{ id: '2', name: 'Han' }],
id: '1',
id: '0',
},
],
hasNext: true,
},
{
completed: [{ id: '1' }],
completed: [{ id: '0' }],
hasNext: false,
},
]);
Expand Down