Skip to content

Commit 2e29180

Browse files
authored
fix: suppress records for deferred fragments that are completely empty (#3984)
i.e. no fields and no enclosed deferred fragments These fragments can be thought of to be completely skipped, because including them will just result in emitting metadata but no actual data. Alternatively, these fragments can be thought of as being inlined. This could probably be considered a bug fix, in that Example F @ graphql/defer-stream-wg#69 explicitly states that these fragments should be skipped.
1 parent 7a6d055 commit 2e29180

File tree

3 files changed

+49
-144
lines changed

3 files changed

+49
-144
lines changed

src/execution/IncrementalPublisher.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,10 +666,13 @@ export class IncrementalPublisher {
666666
return;
667667
}
668668

669-
if (subsequentResultRecord._pending.size === 0) {
670-
this._push(subsequentResultRecord);
671-
} else {
669+
if (subsequentResultRecord._pending.size > 0) {
672670
this._introduce(subsequentResultRecord);
671+
} else if (
672+
subsequentResultRecord.deferredGroupedFieldSetRecords.size > 0 ||
673+
subsequentResultRecord.children.size > 0
674+
) {
675+
this._push(subsequentResultRecord);
673676
}
674677
}
675678

src/execution/__tests__/defer-test.ts

Lines changed: 39 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -394,21 +394,13 @@ describe('Execute: defer directive', () => {
394394
}
395395
`);
396396
const result = await complete(document);
397-
expectJSON(result).toDeepEqual([
398-
{
399-
data: {
400-
hero: {
401-
name: 'Luke',
402-
},
397+
expectJSON(result).toDeepEqual({
398+
data: {
399+
hero: {
400+
name: 'Luke',
403401
},
404-
pending: [{ id: '0', path: ['hero'], label: 'DeferTop' }],
405-
hasNext: true,
406-
},
407-
{
408-
completed: [{ id: '0' }],
409-
hasNext: false,
410402
},
411-
]);
403+
});
412404
});
413405
it('Can defer a fragment that is also not deferred, non-deferred fragment is first', async () => {
414406
const document = parse(`
@@ -423,21 +415,13 @@ describe('Execute: defer directive', () => {
423415
}
424416
`);
425417
const result = await complete(document);
426-
expectJSON(result).toDeepEqual([
427-
{
428-
data: {
429-
hero: {
430-
name: 'Luke',
431-
},
418+
expectJSON(result).toDeepEqual({
419+
data: {
420+
hero: {
421+
name: 'Luke',
432422
},
433-
pending: [{ id: '0', path: ['hero'], label: 'DeferTop' }],
434-
hasNext: true,
435-
},
436-
{
437-
completed: [{ id: '0' }],
438-
hasNext: false,
439423
},
440-
]);
424+
});
441425
});
442426

443427
it('Can defer an inline fragment', async () => {
@@ -481,19 +465,11 @@ describe('Execute: defer directive', () => {
481465
}
482466
`);
483467
const result = await complete(document);
484-
expectJSON(result).toDeepEqual([
485-
{
486-
data: {
487-
hero: {},
488-
},
489-
pending: [{ id: '0', path: ['hero'] }],
490-
hasNext: true,
491-
},
492-
{
493-
completed: [{ id: '0' }],
494-
hasNext: false,
468+
expectJSON(result).toDeepEqual({
469+
data: {
470+
hero: {},
495471
},
496-
]);
472+
});
497473
});
498474

499475
it('Can separately emit defer fragments with different labels with varying fields', async () => {
@@ -775,40 +751,18 @@ describe('Execute: defer directive', () => {
775751
data: { hero: { friends: [{}, {}, {}] } },
776752
pending: [
777753
{ id: '0', path: ['hero', 'friends', 0] },
778-
{ id: '1', path: ['hero', 'friends', 0] },
779-
{ id: '2', path: ['hero', 'friends', 0] },
780-
{ id: '3', path: ['hero', 'friends', 0] },
781-
{ id: '4', path: ['hero', 'friends', 1] },
782-
{ id: '5', path: ['hero', 'friends', 1] },
783-
{ id: '6', path: ['hero', 'friends', 1] },
784-
{ id: '7', path: ['hero', 'friends', 1] },
785-
{ id: '8', path: ['hero', 'friends', 2] },
786-
{ id: '9', path: ['hero', 'friends', 2] },
787-
{ id: '10', path: ['hero', 'friends', 2] },
788-
{ id: '11', path: ['hero', 'friends', 2] },
754+
{ id: '1', path: ['hero', 'friends', 1] },
755+
{ id: '2', path: ['hero', 'friends', 2] },
789756
],
790757
hasNext: true,
791758
},
792759
{
793760
incremental: [
794761
{ data: { id: '2', name: 'Han' }, id: '0' },
795-
{ data: { id: '3', name: 'Leia' }, id: '4' },
796-
{ data: { id: '4', name: 'C-3PO' }, id: '8' },
797-
],
798-
completed: [
799-
{ id: '1' },
800-
{ id: '2' },
801-
{ id: '3' },
802-
{ id: '5' },
803-
{ id: '6' },
804-
{ id: '7' },
805-
{ id: '9' },
806-
{ id: '10' },
807-
{ id: '11' },
808-
{ id: '0' },
809-
{ id: '4' },
810-
{ id: '8' },
762+
{ data: { id: '3', name: 'Leia' }, id: '1' },
763+
{ data: { id: '4', name: 'C-3PO' }, id: '2' },
811764
],
765+
completed: [{ id: '0' }, { id: '1' }, { id: '2' }],
812766
hasNext: false,
813767
},
814768
]);
@@ -1494,21 +1448,13 @@ describe('Execute: defer directive', () => {
14941448
}
14951449
`);
14961450
const result = await complete(document);
1497-
expectJSON(result).toDeepEqual([
1498-
{
1499-
data: {
1500-
hero: {
1501-
friends: [{ name: 'Han' }, { name: 'Leia' }, { name: 'C-3PO' }],
1502-
},
1451+
expectJSON(result).toDeepEqual({
1452+
data: {
1453+
hero: {
1454+
friends: [{ name: 'Han' }, { name: 'Leia' }, { name: 'C-3PO' }],
15031455
},
1504-
pending: [{ id: '0', path: ['hero'] }],
1505-
hasNext: true,
15061456
},
1507-
{
1508-
completed: [{ id: '0' }],
1509-
hasNext: false,
1510-
},
1511-
]);
1457+
});
15121458
});
15131459

15141460
it('Deduplicates async iterable list fields', async () => {
@@ -1534,17 +1480,9 @@ describe('Execute: defer directive', () => {
15341480
},
15351481
},
15361482
});
1537-
expectJSON(result).toDeepEqual([
1538-
{
1539-
data: { hero: { friends: [{ name: 'Han' }] } },
1540-
pending: [{ id: '0', path: ['hero'] }],
1541-
hasNext: true,
1542-
},
1543-
{
1544-
completed: [{ id: '0' }],
1545-
hasNext: false,
1546-
},
1547-
]);
1483+
expectJSON(result).toDeepEqual({
1484+
data: { hero: { friends: [{ name: 'Han' }] } },
1485+
});
15481486
});
15491487

15501488
it('Deduplicates empty async iterable list fields', async () => {
@@ -1571,17 +1509,9 @@ describe('Execute: defer directive', () => {
15711509
},
15721510
},
15731511
});
1574-
expectJSON(result).toDeepEqual([
1575-
{
1576-
data: { hero: { friends: [] } },
1577-
pending: [{ id: '0', path: ['hero'] }],
1578-
hasNext: true,
1579-
},
1580-
{
1581-
completed: [{ id: '0' }],
1582-
hasNext: false,
1583-
},
1584-
]);
1512+
expectJSON(result).toDeepEqual({
1513+
data: { hero: { friends: [] } },
1514+
});
15851515
});
15861516

15871517
it('Does not deduplicate list fields with non-overlapping fields', async () => {
@@ -1655,17 +1585,9 @@ describe('Execute: defer directive', () => {
16551585
friends: () => [],
16561586
},
16571587
});
1658-
expectJSON(result).toDeepEqual([
1659-
{
1660-
data: { hero: { friends: [] } },
1661-
pending: [{ id: '0', path: ['hero'] }],
1662-
hasNext: true,
1663-
},
1664-
{
1665-
completed: [{ id: '0' }],
1666-
hasNext: false,
1667-
},
1668-
]);
1588+
expectJSON(result).toDeepEqual({
1589+
data: { hero: { friends: [] } },
1590+
});
16691591
});
16701592

16711593
it('Deduplicates null object fields', async () => {
@@ -1689,17 +1611,9 @@ describe('Execute: defer directive', () => {
16891611
nestedObject: () => null,
16901612
},
16911613
});
1692-
expectJSON(result).toDeepEqual([
1693-
{
1694-
data: { hero: { nestedObject: null } },
1695-
pending: [{ id: '0', path: ['hero'] }],
1696-
hasNext: true,
1697-
},
1698-
{
1699-
completed: [{ id: '0' }],
1700-
hasNext: false,
1701-
},
1702-
]);
1614+
expectJSON(result).toDeepEqual({
1615+
data: { hero: { nestedObject: null } },
1616+
});
17031617
});
17041618

17051619
it('Deduplicates promise object fields', async () => {
@@ -1722,17 +1636,9 @@ describe('Execute: defer directive', () => {
17221636
nestedObject: () => Promise.resolve({ name: 'foo' }),
17231637
},
17241638
});
1725-
expectJSON(result).toDeepEqual([
1726-
{
1727-
data: { hero: { nestedObject: { name: 'foo' } } },
1728-
pending: [{ id: '0', path: ['hero'] }],
1729-
hasNext: true,
1730-
},
1731-
{
1732-
completed: [{ id: '0' }],
1733-
hasNext: false,
1734-
},
1735-
]);
1639+
expectJSON(result).toDeepEqual({
1640+
data: { hero: { nestedObject: { name: 'foo' } } },
1641+
});
17361642
});
17371643

17381644
it('Handles errors thrown in deferred fragments', async () => {

src/execution/__tests__/stream-test.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,33 +1780,29 @@ describe('Execute: stream directive', () => {
17801780
nestedFriendList: [],
17811781
},
17821782
},
1783-
pending: [
1784-
{ id: '0', path: ['nestedObject'] },
1785-
{ id: '1', path: ['nestedObject', 'nestedFriendList'] },
1786-
],
1783+
pending: [{ id: '0', path: ['nestedObject', 'nestedFriendList'] }],
17871784
hasNext: true,
17881785
},
17891786
{
17901787
incremental: [
17911788
{
17921789
items: [{ id: '1', name: 'Luke' }],
1793-
id: '1',
1790+
id: '0',
17941791
},
17951792
],
1796-
completed: [{ id: '0' }],
17971793
hasNext: true,
17981794
},
17991795
{
18001796
incremental: [
18011797
{
18021798
items: [{ id: '2', name: 'Han' }],
1803-
id: '1',
1799+
id: '0',
18041800
},
18051801
],
18061802
hasNext: true,
18071803
},
18081804
{
1809-
completed: [{ id: '1' }],
1805+
completed: [{ id: '0' }],
18101806
hasNext: false,
18111807
},
18121808
]);

0 commit comments

Comments
 (0)