deduplicate leaf fields present in an initial or parent payload

Author: yaacovCR

src/execution/__tests__/defer-test.ts

@@ -186,7 +186,6 @@ describe('Execute: defer directive', () => {
         }
       }
       fragment NameFragment on Hero {
-        id
         name
       }
     `);
@@ -205,7 +204,6 @@ describe('Execute: defer directive', () => {
         incremental: [
           {
             data: {
-              id: '1',
               name: 'Luke',
             },
             path: ['hero'],
@@ -410,9 +408,7 @@ describe('Execute: defer directive', () => {
       {
         incremental: [
           {
-            data: {
-              name: 'Luke',
-            },
+            data: {},
             path: ['hero'],
           },
         ],
@@ -447,9 +443,7 @@ describe('Execute: defer directive', () => {
       {
         incremental: [
           {
-            data: {
-              name: 'Luke',
-            },
+            data: {},
             path: ['hero'],
           },
         ],
@@ -527,7 +521,7 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
-  it('Does not deduplicate leaf fields present in the initial payload', async () => {
+  it('Can deduplicate leaf fields present in the initial payload', async () => {
     const document = parse(`
       query {
         hero {
@@ -585,6 +579,48 @@ describe('Execute: defer directive', () => {
                 },
               },
               anotherNestedObject: {
+                deeperObject: {},
+              },
+            },
+            path: ['hero'],
+          },
+        ],
+        hasNext: false,
+      },
+    ]);
+  });
+
+  it('Can deduplicate leaf fields present in a parent defer payload', async () => {
+    const document = parse(`
+      query {
+        hero {
+          ... @defer {
+            nestedObject {
+              deeperObject {
+                foo
+                ... @defer {
+                  foo
+                  bar
+                }
+              }
+            }
+          }
+        }
+      }
+    `);
+    const result = await complete(document);
+    expectJSON(result).toDeepEqual([
+      {
+        data: {
+          hero: {},
+        },
+        hasNext: true,
+      },
+      {
+        incremental: [
+          {
+            data: {
+              nestedObject: {
                 deeperObject: {
                   foo: 'foo',
                 },
@@ -593,12 +629,23 @@ describe('Execute: defer directive', () => {
             path: ['hero'],
           },
         ],
+        hasNext: true,
+      },
+      {
+        incremental: [
+          {
+            data: {
+              bar: 'bar',
+            },
+            path: ['hero', 'nestedObject', 'deeperObject'],
+          },
+        ],
         hasNext: false,
       },
     ]);
   });
 
-  it('Does not deduplicate fields with deferred fragments at multiple levels', async () => {
+  it('Does not completely deduplicate fields with deferred fragments at multiple levels', async () => {
     const document = parse(`
       query {
         hero {
@@ -649,7 +696,6 @@ describe('Execute: defer directive', () => {
         incremental: [
           {
             data: {
-              foo: 'foo',
               bar: 'bar',
               baz: 'baz',
               bak: 'bak',
@@ -659,7 +705,6 @@ describe('Execute: defer directive', () => {
           {
             data: {
               deeperObject: {
-                foo: 'foo',
                 bar: 'bar',
                 baz: 'baz',
               },
@@ -670,7 +715,6 @@ describe('Execute: defer directive', () => {
             data: {
               nestedObject: {
                 deeperObject: {
-                  foo: 'foo',
                   bar: 'bar',
                 },
               },

src/execution/collectFields.ts

@@ -14,7 +14,7 @@ import { OperationTypeNode } from '../language/ast.js';
 import { Kind } from '../language/kinds.js';
 
 import type { GraphQLObjectType } from '../type/definition.js';
-import { isAbstractType } from '../type/definition.js';
+import { isAbstractType, isLeafType } from '../type/definition.js';
 import {
   GraphQLDeferDirective,
   GraphQLIncludeDirective,
@@ -42,13 +42,15 @@ import { getDirectiveValues } from './values.js';
 export interface FieldGroup {
   depth: number;
   fields: Map<number | undefined, ReadonlyArray<FieldNode>>;
+  isLeaf: boolean;
 }
 
 export type GroupedFieldSet = Map<string, FieldGroup>;
 
 interface MutableFieldGroup {
   depth: number;
   fields: AccumulatorMap<number | undefined, FieldNode>;
+  isLeaf: boolean;
 }
 
 /**
@@ -166,9 +168,12 @@ function collectFieldsImpl(
         const key = getFieldEntryKey(selection);
         let fieldGroup = groupedFieldSet.get(key);
         if (!fieldGroup) {
+          const fieldDef = schema.getField(runtimeType, selection.name.value);
+          const isLeaf = isLeafType(fieldDef?.type);
           fieldGroup = {
             depth,
             fields: new AccumulatorMap<number | undefined, FieldNode>(),
+            isLeaf,
           };
           groupedFieldSet.set(key, fieldGroup);
         }

src/execution/execute.ts

@@ -658,10 +658,49 @@ function executeFieldsSerially(
 
 function shouldExecute(
   fieldGroup: FieldGroup,
-  deferDepth?: number | undefined,
+  asyncPayload?: AsyncPayloadRecord | undefined,
 ): boolean {
-  const fieldGroupForDeferDepth = fieldGroup.fields.get(deferDepth);
-  return fieldGroupForDeferDepth !== undefined;
+  let deferDepth: number | undefined;
+  if (
+    asyncPayload === undefined ||
+    (deferDepth = asyncPayload.deferDepth) === undefined ||
+    !fieldGroup.isLeaf
+  ) {
+    for (const fieldDeferDepth of fieldGroup.fields.keys()) {
+      if (fieldDeferDepth === deferDepth) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  let hasDepth = false;
+  for (const fieldDeferDepth of fieldGroup.fields.keys()) {
+    if (inDeferTree(asyncPayload, fieldDeferDepth, deferDepth)) {
+      return false;
+    }
+    if (fieldDeferDepth === deferDepth) {
+      hasDepth = true;
+    }
+  }
+  return hasDepth;
+}
+
+function inDeferTree(
+  asyncPayload: AsyncPayloadRecord,
+  fieldDeferDepth: number | undefined,
+  deferDepth: number | undefined,
+): boolean {
+  const parentContext = asyncPayload.parentContext;
+  if (parentContext === undefined) {
+    return fieldDeferDepth === undefined;
+  }
+  const parentDeferDepth = parentContext.deferDepth;
+  // Stream payloads have the same deferDepth as their parent, and so should be skipped.
+  if (parentDeferDepth !== deferDepth && fieldDeferDepth === parentDeferDepth) {
+    return true;
+  }
+  return inDeferTree(parentContext, fieldDeferDepth, parentDeferDepth);
 }
 
 /**
@@ -683,7 +722,7 @@ function executeFields(
     for (const [responseName, fieldGroup] of groupedFieldSet) {
       const fieldPath = exeContext.addPath(path, responseName, parentType.name);
 
-      if (shouldExecute(fieldGroup, asyncPayloadRecord?.deferDepth)) {
+      if (shouldExecute(fieldGroup, asyncPayloadRecord)) {
         const result = executeField(
           exeContext,
           parentType,