Skip to content

Commit

Permalink
deduplicate (mask) leaf values that have already been sent
Browse files Browse the repository at this point in the history
causes some additional minor changes to `hasNext` property on some payloads due to modification of returned promises resulting in an extra tick
  • Loading branch information
yaacovCR committed Jan 12, 2023
1 parent dd3d9e9 commit 3da1cf0
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 20 deletions.
17 changes: 5 additions & 12 deletions src/execution/__tests__/defer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ describe('Execute: defer directive', () => {
]);
});

it('Can deduplicate initial fields with deferred fragments at multiple levels', async () => {
it('Can deduplicate fields with deferred fragments at multiple levels', async () => {
const document = parse(`
query {
hero {
Expand Down Expand Up @@ -650,19 +650,14 @@ describe('Execute: defer directive', () => {
},
{
data: {
deeperObject: {
bar: 'bar',
baz: 'baz',
},
deeperObject: {},
},
path: ['hero', 'nestedObject'],
},
{
data: {
nestedObject: {
deeperObject: {
bar: 'bar',
},
deeperObject: {},
},
},
path: ['hero'],
Expand Down Expand Up @@ -732,7 +727,7 @@ describe('Execute: defer directive', () => {
]);
});

it('can deduplicate initial fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
it('can deduplicate fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
const document = parse(`
query {
a {
Expand Down Expand Up @@ -789,9 +784,7 @@ describe('Execute: defer directive', () => {
data: {
a: {
b: {
e: {
f: 'f',
},
e: {},
},
},
g: {
Expand Down
6 changes: 0 additions & 6 deletions src/execution/__tests__/stream-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1170,9 +1170,6 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand Down Expand Up @@ -1355,9 +1352,6 @@ describe('Execute: stream directive', () => {
],
},
],
hasNext: true,
},
{
hasNext: false,
},
]);
Expand Down
37 changes: 35 additions & 2 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export interface ExecutionContext {
errors: Array<GraphQLError>;
subsequentPayloads: Set<AsyncPayloadRecord>;
branches: WeakMap<GroupedFieldSet, Set<string>>;
leaves: Set<string>;
}

/**
Expand Down Expand Up @@ -503,6 +504,7 @@ export function buildExecutionContext(
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
subsequentPayloads: new Set(),
branches: new WeakMap(),
leaves: new Set(),
errors: [],
};
}
Expand All @@ -516,6 +518,7 @@ function buildPerEventExecutionContext(
rootValue: payload,
subsequentPayloads: new Set(),
branches: new WeakMap(),
leaves: new Set(),
errors: [],
};
}
Expand Down Expand Up @@ -703,6 +706,8 @@ function executeFields(
const results = Object.create(null);
let containsPromise = false;

const shouldMask = Object.create(null);

try {
for (const [responseName, fieldSet] of groupedFieldSet) {
const fieldPath = addPath(path, responseName, parentType.name);
Expand All @@ -716,6 +721,20 @@ function executeFields(
const returnType = fieldDef.type;

if (shouldExecuteFieldSet(returnType, fieldSet, deferDepth)) {
if (
asyncPayloadRecord !== undefined &&
isLeafType(getNamedType(returnType))
) {
shouldMask[responseName] = () => {
const key = pathToArray(fieldPath).join('.');
if (exeContext.leaves.has(key)) {
return true;
}
exeContext.leaves.add(key);
return false;
};
}

const result = executeField(
exeContext,
parentType,
Expand Down Expand Up @@ -748,13 +767,27 @@ function executeFields(

// If there are no promises, we can just return the object
if (!containsPromise) {
return results;
return asyncPayloadRecord === undefined
? results
: new Proxy(results, {
ownKeys: (target) =>
Reflect.ownKeys(target).filter((key) => !shouldMask[key]?.()),
});
}

// Otherwise, results is a map from field name to the result of resolving that
// field, which is possibly a promise. Return a promise that will return this
// same map, but with any promises replaced with the values they resolved to.
return promiseForObject(results);
const promisedResult = promiseForObject(results);
return asyncPayloadRecord === undefined
? promisedResult
: promisedResult.then(
(resolved) =>
new Proxy(resolved, {
ownKeys: (target) =>
Reflect.ownKeys(target).filter((key) => !shouldMask[key]?.()),
}),
);
}

/**
Expand Down

0 comments on commit 3da1cf0

Please sign in to comment.