deduplicate leaf fields from initial payload

graphql · Jan 15, 2023 · 86f9166 · 86f9166
1 parent cdbfbde
commit 86f9166
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 29 deletions.
diff --git a/src/execution/__tests__/defer-test.ts b/src/execution/__tests__/defer-test.ts
@@ -205,7 +205,6 @@ describe('Execute: defer directive', () => {
         incremental: [
           {
             data: {
-              id: '1',
               name: 'Luke',
             },
             path: ['hero'],
@@ -410,9 +409,7 @@ describe('Execute: defer directive', () => {
       {
         incremental: [
           {
-            data: {
-              name: 'Luke',
-            },
+            data: {},
             path: ['hero'],
           },
         ],
@@ -447,9 +444,7 @@ describe('Execute: defer directive', () => {
       {
         incremental: [
           {
-            data: {
-              name: 'Luke',
-            },
+            data: {},
             path: ['hero'],
           },
         ],
@@ -527,7 +522,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,9 +580,7 @@ describe('Execute: defer directive', () => {
                 },
               },
               anotherNestedObject: {
-                deeperObject: {
-                  foo: 'foo',
-                },
+                deeperObject: {},
               },
             },
             path: ['hero'],
@@ -598,7 +591,7 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
-  it('Does not deduplicate fields with deferred fragments at multiple levels', async () => {
+  it('Can deduplicate fields with deferred fragments at multiple levels', async () => {
     const document = parse(`
       query {
         hero {
@@ -649,7 +642,6 @@ describe('Execute: defer directive', () => {
         incremental: [
           {
             data: {
-              foo: 'foo',
               bar: 'bar',
               baz: 'baz',
               bak: 'bak',
@@ -659,7 +651,6 @@ describe('Execute: defer directive', () => {
           {
             data: {
               deeperObject: {
-                foo: 'foo',
                 bar: 'bar',
                 baz: 'baz',
               },
@@ -670,7 +661,6 @@ describe('Execute: defer directive', () => {
             data: {
               nestedObject: {
                 deeperObject: {
-                  foo: 'foo',
                   bar: 'bar',
                 },
               },

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -38,6 +38,7 @@ import type {
   GraphQLTypeResolver,
 } from '../type/definition.js';
 import {
+  getNamedType,
   isAbstractType,
   isLeafType,
   isListType,
@@ -630,19 +631,26 @@ function executeFieldsSerially(
     (results, [responseName, fieldGroup]) => {
       const fieldPath = exeContext.addPath(path, responseName, parentType.name);
 
-      if (!shouldExecute(fieldGroup)) {
+      const fieldName = fieldGroup[0].fieldNode.name.value;
+      const fieldDef = exeContext.schema.getField(parentType, fieldName);
+      if (!fieldDef) {
+        return results;
+      }
+
+      const returnType = fieldDef.type;
+
+      if (!shouldExecute(fieldGroup, returnType)) {
         return results;
       }
       const result = executeField(
         exeContext,
         parentType,
+        fieldDef,
+        returnType,
         sourceValue,
         fieldGroup,
         fieldPath,
       );
-      if (result === undefined) {
-        return results;
-      }
       if (isPromise(result)) {
         return result.then((resolvedResult) => {
           results[responseName] = resolvedResult;
@@ -658,11 +666,25 @@ function executeFieldsSerially(
 
 function shouldExecute(
   fieldGroup: FieldGroup,
+  returnType: GraphQLOutputType,
   deferDepth?: number | undefined,
 ): boolean {
-  return fieldGroup.some(
-    ({ deferDepth: fieldDeferDepth }) => fieldDeferDepth === deferDepth,
-  );
+  if (deferDepth === undefined || !isLeafType(getNamedType(returnType))) {
+    return fieldGroup.some(
+      ({ deferDepth: fieldDeferDepth }) => fieldDeferDepth === deferDepth,
+    );
+  }
+
+  let hasDepth = false;
+  for (const { deferDepth: fieldDeferDepth } of fieldGroup) {
+    if (fieldDeferDepth === undefined) {
+      return false;
+    }
+    if (fieldDeferDepth === deferDepth) {
+      hasDepth = true;
+    }
+  }
+  return hasDepth;
 }
 
 /**
@@ -684,10 +706,22 @@ function executeFields(
     for (const [responseName, fieldGroup] of groupedFieldSet) {
       const fieldPath = exeContext.addPath(path, responseName, parentType.name);
 
-      if (shouldExecute(fieldGroup, asyncPayloadRecord?.deferDepth)) {
+      const fieldName = fieldGroup[0].fieldNode.name.value;
+      const fieldDef = exeContext.schema.getField(parentType, fieldName);
+      if (!fieldDef) {
+        continue;
+      }
+
+      const returnType = fieldDef.type;
+
+      if (
+        shouldExecute(fieldGroup, returnType, asyncPayloadRecord?.deferDepth)
+      ) {
         const result = executeField(
           exeContext,
           parentType,
+          fieldDef,
+          returnType,
           sourceValue,
           fieldGroup,
           fieldPath,
@@ -735,19 +769,15 @@ function toNodes(fieldGroup: FieldGroup): ReadonlyArray<FieldNode> {
 function executeField(
   exeContext: ExecutionContext,
   parentType: GraphQLObjectType,
+  fieldDef: GraphQLField<unknown, unknown>,
+  returnType: GraphQLOutputType,
   source: unknown,
   fieldGroup: FieldGroup,
   path: Path,
   asyncPayloadRecord?: AsyncPayloadRecord,
 ): PromiseOrValue<unknown> {
   const errors = asyncPayloadRecord?.errors ?? exeContext.errors;
-  const fieldName = fieldGroup[0].fieldNode.name.value;
-  const fieldDef = exeContext.schema.getField(parentType, fieldName);
-  if (!fieldDef) {
-    return;
-  }
 
-  const returnType = fieldDef.type;
   const resolveFn = fieldDef.resolve ?? exeContext.fieldResolver;
 
   const info = buildResolveInfo(