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
graphql · Jan 12, 2023 · f15c6aa · f15c6aa
1 parent dd3d9e9
commit f15c6aa
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 24 deletions.
diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts
@@ -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 {
@@ -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'],
@@ -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 {
@@ -789,9 +784,7 @@ describe('Execute: defer directive', () => {
  data: {
  a: {
  b: {
- e: {
- f: 'f',
- },
+ e: {},
  },
  },
  g: {

diff --git a/src/execution/__tests__/stream-test.ts b/src/execution/__tests__/stream-test.ts
@@ -1170,9 +1170,6 @@ describe('Execute: stream directive', () => {
  ],
  },
  ],
- hasNext: true,
- },
- {
  hasNext: false,
  },
  ]);
@@ -1355,9 +1352,6 @@ describe('Execute: stream directive', () => {
  ],
  },
  ],
- hasNext: true,
- },
- {
  hasNext: false,
  },
  ]);

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -124,6 +124,7 @@ export interface ExecutionContext {
  errors: Array<GraphQLError>;
  subsequentPayloads: Set<AsyncPayloadRecord>;
  branches: WeakMap<GroupedFieldSet, Set<string>>;
+ leaves: Set<string>;
 }
 
 /**
@@ -503,6 +504,7 @@ export function buildExecutionContext(
  subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
  subsequentPayloads: new Set(),
  branches: new WeakMap(),
+ leaves: new Set(),
  errors: [],
  };
 }
@@ -516,6 +518,7 @@ function buildPerEventExecutionContext(
  rootValue: payload,
  subsequentPayloads: new Set(),
  branches: new WeakMap(),
+ leaves: new Set(),
  errors: [],
  };
 }
@@ -638,7 +641,9 @@ function executeFieldsSerially(
 
  const returnType = fieldDef.type;
 
- if (!shouldExecuteFieldSet(returnType, fieldSet, undefined)) {
+ const isLeaf = isLeafType(getNamedType(returnType));
+
+ if (!shouldExecuteFieldSet(fieldSet, isLeaf, undefined)) {
  return results;
  }
  const result = executeField(
@@ -665,11 +670,11 @@ function executeFieldsSerially(
 }
 
 function shouldExecuteFieldSet(
- returnType: GraphQLOutputType,
  fieldSet: ReadonlyArray<TaggedFieldNode>,
+ isLeaf: boolean,
  deferDepth: number | undefined,
 ): boolean {
- if (deferDepth === undefined || !isLeafType(getNamedType(returnType))) {
+ if (deferDepth === undefined || !isLeaf) {
  return fieldSet.some(
  ({ deferDepth: fieldDeferDepth }) => fieldDeferDepth === deferDepth,
  );
@@ -703,6 +708,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);
@@ -715,7 +722,23 @@ function executeFields(
 
  const returnType = fieldDef.type;
 
- if (shouldExecuteFieldSet(returnType, fieldSet, deferDepth)) {
+ const isLeaf = isLeafType(getNamedType(returnType));
+
+ if (shouldExecuteFieldSet(fieldSet, isLeaf, 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,
@@ -748,13 +771,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]?.()),
+ }),
+ );
 }
 
 /**