Repository: graphql/graphql-js
Pull request #3728 (2022-09-07 19:19:08)

incrementalDelivery: refactoring and streamlining
Each commit is meant to merged separately, i.e. "Rebase and Merge" rather than "Squash and Merge"

419fd37f: moves `executeStreamField` out of try block to better illustrate which code might throw
2991c79a: tiny refactor pushing the result of `handleFieldError` rather than explicit null, following convention elsewhere
71a8db4f: rename `fieldPath` to `itemPath` when dealing with items, as elsewhere
4df02ca7: skip unnecessary filtering when asyncIterator's next method throws, aligning stream to non-stream

netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy Preview for *compassionate-pike-271cb3* processing.


|  Name | Link |
|---------------------------------|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 437fa09cab47ca9700a27e92a3ddf6ffd9336428 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/6318eeadc2c1940009e80ac5 |
yaacovCR: this will have to come after #3746 and #3747 
yaacovCR: `completeListItemValue` has been separated out into #3728 -- this is now ready for merge.

Patch:
diff --git a/src/execution/execute.ts b/src/execution/execute.ts
index 5e19e662..c3b1e358 100644
--- a/src/execution/execute.ts
+++ b/src/execution/execute.ts
@@ -996,7 +996,7 @@ async function completeAsyncIteratorValue(
       break;
     }
 
-    const fieldPath = addPath(path, index, undefined);
+    const itemPath = addPath(path, index, undefined);
     try {
       // eslint-disable-next-line no-await-in-loop
       const { value, done } = await iterator.next();
@@ -1011,7 +1011,7 @@ async function completeAsyncIteratorValue(
           itemType,
           fieldNodes,
           info,
-          fieldPath,
+          itemPath,
           value,
           asyncPayloadRecord,
         );
@@ -1024,10 +1024,10 @@ async function completeAsyncIteratorValue(
               const error = locatedError(
                 rawError,
                 fieldNodes,
-                pathToArray(fieldPath),
+                pathToArray(itemPath),
               );
               const handledError = handleFieldError(error, itemType, errors);
-              filterSubsequentPayloads(exeContext, fieldPath);
+              filterSubsequentPayloads(exeContext, itemPath);
               return handledError;
             }),
           );
@@ -1036,18 +1036,13 @@ async function completeAsyncIteratorValue(
         }
       } catch (rawError) {
         completedResults.push(null);
-        const error = locatedError(
-          rawError,
-          fieldNodes,
-          pathToArray(fieldPath),
-        );
-        filterSubsequentPayloads(exeContext, fieldPath);
+        const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
+        filterSubsequentPayloads(exeContext, itemPath);
         handleFieldError(error, itemType, errors);
       }
     } catch (rawError) {
-      completedResults.push(null);
-      const error = locatedError(rawError, fieldNodes, pathToArray(fieldPath));
-      handleFieldError(error, itemType, errors);
+      const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
+      completedResults.push(handleFieldError(error, itemType, errors));
       break;
     }
     index += 1;
@@ -1103,28 +1098,29 @@ function completeListValue(
     // No need to modify the info object containing the path,
     // since from here on it is not ever accessed by resolver functions.
     const itemPath = addPath(path, index, undefined);
+
+    if (
+      stream &&
+      typeof stream.initialCount === 'number' &&
+      index >= stream.initialCount
+    ) {
+      previousAsyncPayloadRecord = executeStreamField(
+        path,
+        itemPath,
+        item,
+        exeContext,
+        fieldNodes,
+        info,
+        itemType,
+        stream.label,
+        previousAsyncPayloadRecord,
+      );
+      index++;
+      continue;
+    }
+
     try {
       let completedItem;
-
-      if (
-        stream &&
-        typeof stream.initialCount === 'number' &&
-        index >= stream.initialCount
-      ) {
-        previousAsyncPayloadRecord = executeStreamField(
-          path,
-          itemPath,
-          item,
-          exeContext,
-          fieldNodes,
-          info,
-          itemType,
-          stream.label,
-          previousAsyncPayloadRecord,
-        );
-        index++;
-        continue;
-      }
       if (isPromise(item)) {
         completedItem = item.then((resolved) =>
           completeValue(
@@ -1939,7 +1935,7 @@ async function executeStreamIteratorItem(
   info: GraphQLResolveInfo,
   itemType: GraphQLOutputType,
   asyncPayloadRecord: StreamRecord,
-  fieldPath: Path,
+  itemPath: Path,
 ): Promise<IteratorResult<unknown>> {
   let item;
   try {
@@ -1950,9 +1946,8 @@ async function executeStreamIteratorItem(
     }
     item = value;
   } catch (rawError) {
-    const error = locatedError(rawError, fieldNodes, pathToArray(fieldPath));
+    const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
     const value = handleFieldError(error, itemType, asyncPayloadRecord.errors);
-    filterSubsequentPayloads(exeContext, fieldPath, asyncPayloadRecord);
     // don't continue if iterator throws
     return { done: true, value };
   }
@@ -1963,32 +1958,28 @@ async function executeStreamIteratorItem(
       itemType,
       fieldNodes,
       info,
-      fieldPath,
+      itemPath,
       item,
       asyncPayloadRecord,
     );
 
     if (isPromise(completedItem)) {
       completedItem = completedItem.then(undefined, (rawError) => {
-        const error = locatedError(
-          rawError,
-          fieldNodes,
-          pathToArray(fieldPath),
-        );
+        const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
         const handledError = handleFieldError(
           error,
           itemType,
           asyncPayloadRecord.errors,
         );
-        filterSubsequentPayloads(exeContext, fieldPath, asyncPayloadRecord);
+        filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord);
         return handledError;
       });
     }
     return { done: false, value: completedItem };
   } catch (rawError) {
-    const error = locatedError(rawError, fieldNodes, pathToArray(fieldPath));
+    const error = locatedError(rawError, fieldNodes, pathToArray(itemPath));
     const value = handleFieldError(error, itemType, asyncPayloadRecord.errors);
-    filterSubsequentPayloads(exeContext, fieldPath, asyncPayloadRecord);
+    filterSubsequentPayloads(exeContext, itemPath, asyncPayloadRecord);
     return { done: false, value };
   }
 }
@@ -2008,10 +1999,10 @@ async function executeStreamIterator(
   let previousAsyncPayloadRecord = parentContext ?? undefined;
   // eslint-disable-next-line no-constant-condition
   while (true) {
-    const fieldPath = addPath(path, index, undefined);
+    const itemPath = addPath(path, index, undefined);
     const asyncPayloadRecord = new StreamRecord({
       label,
-      path: fieldPath,
+      path: itemPath,
       parentContext: previousAsyncPayloadRecord,
       iterator,
       exeContext,
@@ -2024,7 +2015,7 @@ async function executeStreamIterator(
       info,
       itemType,
       asyncPayloadRecord,
-      fieldPath,
+      itemPath,
     );
 
     asyncPayloadRecord.addItems(
diff --git a/src/language/parser.ts b/src/language/parser.ts
index 9eef51e3..bc58875e 100644
--- a/src/language/parser.ts
+++ b/src/language/parser.ts
@@ -1638,7 +1638,7 @@ export class Parser {
         throw syntaxError(
           this._lexer.source,
           token.start,
-          `Document contains more that ${maxTokens} tokens. Parsing aborted.`,
+          `Document contains more than ${maxTokens} tokens. Parsing aborted.`,
         );
       }
     }


Test patch:
diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts
index 7c988a01..34e9dff4 100644
--- a/src/language/__tests__/parser-test.ts
+++ b/src/language/__tests__/parser-test.ts
@@ -91,16 +91,16 @@ describe('Parser', () => {
     `);
   });
 
-  it('limit maximum number of tokens', () => {
+  it('limits by a maximum number of tokens', () => {
     expect(() => parse('{ foo }', { maxTokens: 3 })).to.not.throw();
     expect(() => parse('{ foo }', { maxTokens: 2 })).to.throw(
-      'Syntax Error: Document contains more that 2 tokens. Parsing aborted.',
+      'Syntax Error: Document contains more than 2 tokens. Parsing aborted.',
     );
 
     expect(() => parse('{ foo(bar: "baz") }', { maxTokens: 8 })).to.not.throw();
 
     expect(() => parse('{ foo(bar: "baz") }', { maxTokens: 7 })).to.throw(
-      'Syntax Error: Document contains more that 7 tokens. Parsing aborted.',
+      'Syntax Error: Document contains more than 7 tokens. Parsing aborted.',
     );
   });