Skip to content

Commit 0763c59

Browse files
committed
deduplicate (mask) leaf values that have already been sent
causes some additional minor changes to `hasNext` property on some payloads due to modification of returned promises resulting in an extra tick
1 parent d34e5d4 commit 0763c59

File tree

3 files changed

+50
-24
lines changed

3 files changed

+50
-24
lines changed

src/execution/__tests__/defer-test.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -650,19 +650,14 @@ describe('Execute: defer directive', () => {
650650
},
651651
{
652652
data: {
653-
deeperObject: {
654-
bar: 'bar',
655-
baz: 'baz',
656-
},
653+
deeperObject: {},
657654
},
658655
path: ['hero', 'nestedObject'],
659656
},
660657
{
661658
data: {
662659
nestedObject: {
663-
deeperObject: {
664-
bar: 'bar',
665-
},
660+
deeperObject: {},
666661
},
667662
},
668663
path: ['hero'],
@@ -789,9 +784,7 @@ describe('Execute: defer directive', () => {
789784
data: {
790785
a: {
791786
b: {
792-
e: {
793-
f: 'f',
794-
},
787+
e: {},
795788
},
796789
},
797790
g: {

src/execution/__tests__/stream-test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,9 +1170,6 @@ describe('Execute: stream directive', () => {
11701170
],
11711171
},
11721172
],
1173-
hasNext: true,
1174-
},
1175-
{
11761173
hasNext: false,
11771174
},
11781175
]);
@@ -1355,9 +1352,6 @@ describe('Execute: stream directive', () => {
13551352
],
13561353
},
13571354
],
1358-
hasNext: true,
1359-
},
1360-
{
13611355
hasNext: false,
13621356
},
13631357
]);

src/execution/execute.ts

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export interface ExecutionContext {
126126
errors: Array<GraphQLError>;
127127
subsequentPayloads: Set<AsyncPayloadRecord>;
128128
branches: WeakMap<GroupedFieldSet, Set<Path | undefined>>;
129+
leaves: WeakMap<GroupedFieldSet, Set<Path | undefined>>;
129130
}
130131

131132
/**
@@ -506,6 +507,7 @@ export function buildExecutionContext(
506507
addPath: createPathFactory(),
507508
subsequentPayloads: new Set(),
508509
branches: new WeakMap(),
510+
leaves: new WeakMap(),
509511
errors: [],
510512
};
511513
}
@@ -520,6 +522,7 @@ function buildPerEventExecutionContext(
520522
addPath: createPathFactory(),
521523
subsequentPayloads: new Set(),
522524
branches: new WeakMap(),
525+
leaves: new WeakMap(),
523526
errors: [],
524527
};
525528
}
@@ -639,7 +642,9 @@ function executeFieldsSerially(
639642

640643
const returnType = fieldDef.type;
641644

642-
if (!shouldExecute(fieldGroup, returnType)) {
645+
const isLeaf = isLeafType(getNamedType(returnType));
646+
647+
if (!shouldExecute(fieldGroup, isLeaf)) {
643648
return results;
644649
}
645650
const result = executeField(
@@ -666,10 +671,10 @@ function executeFieldsSerially(
666671

667672
function shouldExecute(
668673
fieldGroup: FieldGroup,
669-
returnType: GraphQLOutputType,
674+
isLeaf: boolean,
670675
deferDepth?: number | undefined,
671676
): boolean {
672-
if (deferDepth === undefined || !isLeafType(getNamedType(returnType))) {
677+
if (deferDepth === undefined || !isLeaf) {
673678
return fieldGroup.some(
674679
({ deferDepth: fieldDeferDepth }) => fieldDeferDepth === deferDepth,
675680
);
@@ -702,6 +707,8 @@ function executeFields(
702707
const results = Object.create(null);
703708
let containsPromise = false;
704709

710+
const shouldMask = Object.create(null);
711+
705712
try {
706713
for (const [responseName, fieldGroup] of groupedFieldSet) {
707714
const fieldPath = exeContext.addPath(path, responseName, parentType.name);
@@ -714,9 +721,27 @@ function executeFields(
714721

715722
const returnType = fieldDef.type;
716723

717-
if (
718-
shouldExecute(fieldGroup, returnType, asyncPayloadRecord?.deferDepth)
719-
) {
724+
const isLeaf = isLeafType(getNamedType(returnType));
725+
726+
if (shouldExecute(fieldGroup, isLeaf, asyncPayloadRecord?.deferDepth)) {
727+
if (
728+
asyncPayloadRecord !== undefined &&
729+
isLeafType(getNamedType(returnType))
730+
) {
731+
shouldMask[responseName] = () => {
732+
const set = exeContext.leaves.get(groupedFieldSet);
733+
if (set === undefined) {
734+
exeContext.leaves.set(groupedFieldSet, new Set([fieldPath]));
735+
return false;
736+
}
737+
if (set.has(fieldPath)) {
738+
return true;
739+
}
740+
set.add(fieldPath);
741+
return false;
742+
};
743+
}
744+
720745
const result = executeField(
721746
exeContext,
722747
parentType,
@@ -748,13 +773,27 @@ function executeFields(
748773

749774
// If there are no promises, we can just return the object
750775
if (!containsPromise) {
751-
return results;
776+
return asyncPayloadRecord === undefined
777+
? results
778+
: new Proxy(results, {
779+
ownKeys: (target) =>
780+
Reflect.ownKeys(target).filter((key) => !shouldMask[key]?.()),
781+
});
752782
}
753783

754784
// Otherwise, results is a map from field name to the result of resolving that
755785
// field, which is possibly a promise. Return a promise that will return this
756786
// same map, but with any promises replaced with the values they resolved to.
757-
return promiseForObject(results);
787+
const promisedResult = promiseForObject(results);
788+
return asyncPayloadRecord === undefined
789+
? promisedResult
790+
: promisedResult.then(
791+
(resolved) =>
792+
new Proxy(resolved, {
793+
ownKeys: (target) =>
794+
Reflect.ownKeys(target).filter((key) => !shouldMask[key]?.()),
795+
}),
796+
);
758797
}
759798

760799
function toNodes(fieldGroup: FieldGroup): ReadonlyArray<FieldNode> {

0 commit comments

Comments
 (0)