Feedback from open incremental PRs (#13010)

Co-authored-by: Lenz Weber-Tronic <[email protected]>

Author: jerelmiller

.api-reports/api-report-utilities_internal.api.md

@@ -109,20 +109,28 @@ export namespace DeepMerger {
     // (undocumented)
     export type ArrayMergeStrategy = "truncate" | "combine";
     // (undocumented)
+    export interface MergeOptions {
+        // (undocumented)
+        atPath?: ReadonlyArray<string | number>;
+    }
+    // (undocumented)
     export interface Options {
         // (undocumented)
         arrayMerge?: DeepMerger.ArrayMergeStrategy;
+        // Warning: (ae-forgotten-export) The symbol "ReconcilerFunction" needs to be exported by the entry point index.d.ts
+        //
+        // (undocumented)
+        reconciler?: ReconcilerFunction;
     }
 }
 
 // @internal @deprecated (undocumented)
-export class DeepMerger<TContextArgs extends any[] = any[]> {
-    // Warning: (ae-forgotten-export) The symbol "ReconcilerFunction" needs to be exported by the entry point index.d.ts
-    constructor(reconciler?: ReconcilerFunction<TContextArgs>, options?: DeepMerger.Options);
+export class DeepMerger {
+    constructor(options?: DeepMerger.Options);
     // (undocumented)
     isObject: typeof isNonNullObject;
     // (undocumented)
-    merge(target: any, source: any, ...context: TContextArgs): any;
+    merge(target: any, source: any, mergeOptions?: DeepMerger.MergeOptions): any;
     // (undocumented)
     shallowCopyForMerge<T>(value: T): T;
 }
@@ -400,7 +408,7 @@ export type Primitive = null | undefined | string | number | boolean | symbol |
 // Warning: (ae-incompatible-release-tags) The symbol "ReconcilerFunction" is marked as @public, but its signature references "DeepMerger" which is marked as @internal
 //
 // @public (undocumented)
-type ReconcilerFunction<TContextArgs extends any[]> = (this: DeepMerger<TContextArgs>, target: Record<string | number, any>, source: Record<string | number, any>, property: string | number, ...context: TContextArgs) => any;
+type ReconcilerFunction = (this: DeepMerger, target: Record<string | number, any>, source: Record<string | number, any>, property: string | number) => any;
 
 // Warning: (ae-forgotten-export) The symbol "globalCaches" needs to be exported by the entry point index.d.ts
 //

.changeset/early-flies-thank.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix an issue where errors parsed from incremental chunks in `ErrorLink` might throw when using the `GraphQL17Alpha9Handler`.

.changeset/tricky-cycles-buy.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Handle `@stream` payloads that send multiple items in the same chunk when using the `Defer20220824Handler`.

.changeset/wicked-dingos-approve.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Handle an edge case with the `Defer20220824Handler` where an error for a `@stream` item that bubbles to the `@stream` boundary (such as an item returning `null` for a non-null array item) would write items from future chunks to the wrong array index. In these cases, the `@stream` field is no longer processed and future updates to the field are ignored. This prevents runtime errors that TypeScript would otherwise not be able to catch.

src/cache/inmemory/entityStore.ts

@@ -131,10 +131,9 @@ export abstract class EntityStore implements NormalizedCache {
 
     invariant(typeof dataId === "string", "store.merge expects a string ID");
 
-    const merged: StoreObject = new DeepMerger(storeObjectReconciler).merge(
-      existing,
-      incoming
-    );
+    const merged: StoreObject = new DeepMerger({
+      reconciler: storeObjectReconciler,
+    }).merge(existing, incoming);
 
     // Even if merged === existing, existing may have come from a lower
     // layer, so we always need to set this.data[dataId] on this level.

src/cache/inmemory/readFromStore.ts

@@ -414,7 +414,7 @@ export class StoreReader {
     context,
   }: ExecSubSelectedArrayOptions): ExecResult {
     let missing: MissingTree | undefined;
-    let missingMerger = new DeepMerger<MissingTree[]>();
+    let missingMerger = new DeepMerger();
 
     function handleMissing<T>(childResult: ExecResult<T>, i: number): T {
       if (childResult.missing) {

src/core/ObservableQuery.ts

@@ -894,20 +894,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
       { shouldEmit: EmitBehavior.networkStatusChange }
     );
 
-    const { promise, operator } = getTrackingOperatorPromise(
-      (value: QueryNotification.Value<TFetchData>) => {
-        switch (value.kind) {
-          case "E": {
-            throw value.error;
-          }
-          case "N": {
-            if (value.source !== "newNetworkStatus" && !value.value.loading) {
-              return value.value;
-            }
-          }
-        }
-      }
-    );
+    const { promise, operator } = getTrackingOperatorPromise<TFetchData>();
 
     const { observable } = this.queryManager.fetchObservableWithInfo(
       combinedOptions,
@@ -1055,12 +1042,12 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
         .then((result) => toQueryResult(this.maskResult(result)))
         .finally(() => {
           subscription.unsubscribe();
-          if (isCached && !wasUpdated) {
-            finalize();
+          finalize();
 
+          if (isCached && !wasUpdated) {
             const lastResult = this.getCurrentResult();
 
-            if (lastResult.networkStatus === NetworkStatus.streaming) {
+            if (lastResult.dataState === "streaming") {
               pushNotification({
                 kind: "N",
                 source: "network",
@@ -1610,15 +1597,6 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
 
     this.resubscribeCache();
     const { promise, operator: promiseOperator } = getTrackingOperatorPromise(
-      (value: QueryNotification.Value<TData>) => {
-        switch (value.kind) {
-          case "E":
-            throw value.error;
-          case "N":
-            if (value.source !== "newNetworkStatus" && !value.value.loading)
-              return value.value;
-        }
-      },
       // This default value should only be used when using a `fetchPolicy` of
       // `standby` since that fetch policy completes without emitting a
       // result. Since we are converting this to a QueryResult type, we
@@ -2039,46 +2017,49 @@ function isEqualQuery(
   return !!(a && b && a.query === b.query && equal(a.variables, b.variables));
 }
 
-function getTrackingOperatorPromise<ObservedValue, ReturnValue = ObservedValue>(
-  filterMapCb: (value: ObservedValue) => ReturnValue | undefined,
-  defaultValue?: ReturnValue
+function getTrackingOperatorPromise<TData>(
+  defaultValue?: ObservableQuery.Result<TData>
 ) {
   let lastValue = defaultValue,
-    resolve: (value: ReturnValue) => void,
+    resolve: (value: ObservableQuery.Result<TData>) => void,
     reject: (error: unknown) => void;
-  const promise = new Promise<ReturnValue>((res, rej) => {
+  const promise = new Promise<ObservableQuery.Result<TData>>((res, rej) => {
     resolve = res;
     reject = rej;
   });
-  const operator: MonoTypeOperatorFunction<ObservedValue> = tap({
-    next(value) {
-      try {
-        const newValue = filterMapCb(value);
-        if (newValue !== undefined) {
-          lastValue = newValue;
+  const operator: MonoTypeOperatorFunction<QueryNotification.Value<TData>> =
+    tap({
+      next(value) {
+        if (value.kind === "E") {
+          return reject(value.error);
         }
-      } catch (error) {
-        reject(error);
-      }
-    },
-    finalize: () => {
-      if (lastValue) {
-        resolve(lastValue);
-      } else {
-        const message = "The operation was aborted.";
-        const name = "AbortError";
-        reject(
-          typeof DOMException !== "undefined" ?
-            new DOMException(message, name)
-            // some environments do not have `DOMException`, e.g. node
-            // uses a normal `Error` with a `name` property instead: https://github.com/phryneas/node/blob/d0579b64f0f6b722f8e49bf8a471dd0d0604a21e/lib/internal/errors.js#L964
-            // error.code is a legacy property that is not used anymore,
-            // and also inconsistent across environments (in supporting
-            // browsers it is `20`, in node `'ABORT_ERR'`) so we omit that.
-          : Object.assign(new Error(message), { name })
-        );
-      }
-    },
-  });
+
+        if (
+          value.kind === "N" &&
+          value.source !== "newNetworkStatus" &&
+          !value.value.loading
+        ) {
+          lastValue = value.value;
+        }
+      },
+      finalize: () => {
+        if (lastValue) {
+          resolve(lastValue);
+        } else {
+          const message = "The operation was aborted.";
+          const name = "AbortError";
+          reject(
+            typeof DOMException !== "undefined" ?
+              new DOMException(message, name)
+              // some environments do not have `DOMException`, e.g. node
+              // uses a normal `Error` with a `name` property instead: https://github.com/phryneas/node/blob/d0579b64f0f6b722f8e49bf8a471dd0d0604a21e/lib/internal/errors.js#L964
+              // error.code is a legacy property that is not used anymore,
+              // and also inconsistent across environments (in supporting
+              // browsers it is `20`, in node `'ABORT_ERR'`) so we omit that.
+            : Object.assign(new Error(message), { name })
+          );
+        }
+      },
+    });
   return { promise, operator };
 }