fix: proxy defer resolve/reject values through tracked promises (#9200)

* fix: proxy defer resolve/reject values through tracked promises

* Add changeset

* remove blank line

* Simplify approach

* add AbortedDeferredError class

* race against abort signal

* bump, bump, bump it up

Author: brophdawg11

.changeset/young-pumas-destroy.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+fix: proxy defer resolve/reject values through tracked promises (#9200)

package.json

@@ -101,7 +101,7 @@
   },
   "filesize": {
     "packages/router/dist/router.js": {
-      "none": "99 kB"
+      "none": "100 kB"
     },
     "packages/react-router/dist/react-router.production.min.js": {
       "none": "12 kB"

packages/router/__tests__/router-test.ts

@@ -28,7 +28,7 @@ import {
 } from "../index";
 
 // Private API
-import { TrackedPromise } from "../utils";
+import { AbortedDeferredError, TrackedPromise } from "../utils";
 
 ///////////////////////////////////////////////////////////////////////////////
 //#region Types and Utils
@@ -2948,7 +2948,7 @@ describe("a router", () => {
         expect(t.router.state.errors).toBe(null);
         expect(A.loaders.parent.stub.mock.calls.length).toBe(1); // called again for revalidation
         expect(A.loaders.child.stub.mock.calls.length).toBe(1); // called because it's above error
-        expect(A.loaders.grandchild.stub.mock.calls.length).toBe(0); // dont call due to error
+        expect(A.loaders.grandchild.stub.mock.calls.length).toBe(0); // don't call due to error
         await A.loaders.parent.resolve("PARENT DATA*");
         await A.loaders.child.resolve("CHILD DATA");
         expect(t.router.state.loaderData).toEqual({
@@ -7637,6 +7637,12 @@ describe("a router", () => {
           lazy3: expect.trackedPromise("3"),
         },
       });
+
+      // Should proxy values through
+      let data = t.router.state.loaderData.lazy;
+      await expect(data.lazy1).resolves.toBe("Immediate data");
+      await expect(data.lazy2).resolves.toBe("2");
+      await expect(data.lazy3).resolves.toBe("3");
     });
 
     it("should cancel outstanding deferreds on a new navigation", async () => {
@@ -7670,7 +7676,17 @@ describe("a router", () => {
       );
 
       // Interrupt pending deferred's from /lazy navigation
-      let B = await t.navigate("/");
+      let navPromise = t.navigate("/");
+
+      // Cancelled promises should reject immediately
+      let data = t.router.state.loaderData.lazy;
+      await expect(data.lazy1).rejects.toBeInstanceOf(AbortedDeferredError);
+      await expect(data.lazy2).rejects.toBeInstanceOf(AbortedDeferredError);
+      await expect(data.lazy1).rejects.toThrowError("Deferred data aborted");
+      await expect(data.lazy2).rejects.toThrowError("Deferred data aborted");
+
+      let B = await navPromise;
+
       // During navigation - deferreds remain as promises
       expect(t.router.state.loaderData).toEqual({
         lazy: {
@@ -7817,6 +7833,10 @@ describe("a router", () => {
           lazy: expect.trackedPromise(undefined, new Error("Kaboom!")),
         },
       });
+
+      // should proxy the error through
+      let data = t.router.state.loaderData.lazy;
+      await expect(data.lazy).rejects.toEqual(new Error("Kaboom!"));
     });
 
     it("should cancel all outstanding deferreds on router.revalidate()", async () => {
@@ -8941,7 +8961,7 @@ describe("a router", () => {
           lazy: dfd.promise,
         })
       );
-      await dfd.reject(new Error("Kaboom!")).catch(() => {});
+      await dfd.reject(new Error("Kaboom!"));
       expect(t.router.state.errors).toMatchObject({
         index: new Error("Kaboom!"),
       });

packages/router/index.ts

@@ -35,6 +35,7 @@ export type {
 } from "./utils";
 
 export {
+  AbortedDeferredError,
   ErrorResponse,
   defer,
   generatePath,

packages/router/utils.ts

@@ -895,9 +895,13 @@ export interface TrackedPromise extends Promise<any> {
   _error?: any;
 }
 
+export class AbortedDeferredError extends Error {}
+
 export class DeferredData {
   private pendingKeys: Set<string | number> = new Set<string | number>();
-  private cancelled: boolean = false;
+  private controller: AbortController;
+  private abortPromise: Promise<void>;
+  private unlistenAbortSignal: () => void;
   private subscriber?: (aborted: boolean) => void = undefined;
   data: Record<string, unknown>;
 
@@ -906,6 +910,18 @@ export class DeferredData {
       data && typeof data === "object" && !Array.isArray(data),
       "defer() only accepts plain objects"
     );
+
+    // Set up an AbortController + Promise we can race against to exit early
+    // cancellation
+    let reject: (e: AbortedDeferredError) => void;
+    this.abortPromise = new Promise((_, r) => (reject = r));
+    this.controller = new AbortController();
+    let onAbort = () =>
+      reject(new AbortedDeferredError("Deferred data aborted"));
+    this.unlistenAbortSignal = () =>
+      this.controller.signal.removeEventListener("abort", onAbort);
+    this.controller.signal.addEventListener("abort", onAbort);
+
     this.data = Object.entries(data).reduce(
       (acc, [key, value]) =>
         Object.assign(acc, {
@@ -927,10 +943,15 @@ export class DeferredData {
 
     // We store a little wrapper promise that will be extended with
     // _data/_error props upon resolve/reject
-    let promise: TrackedPromise = value.then(
+    let promise: TrackedPromise = Promise.race([value, this.abortPromise]).then(
       (data) => this.onSettle(promise, key, null, data as unknown),
       (error) => this.onSettle(promise, key, error as unknown)
     );
+
+    // Register rejection listeners to avoid uncaught promise rejections on
+    // errors or aborted deferred values
+    promise.catch(() => {});
+
     Object.defineProperty(promise, "_tracked", { get: () => true });
     return promise;
   }
@@ -940,27 +961,39 @@ export class DeferredData {
     key: string | number,
     error: unknown,
     data?: unknown
-  ): void {
-    if (this.cancelled) {
-      return;
+  ): unknown {
+    if (
+      this.controller.signal.aborted &&
+      error instanceof AbortedDeferredError
+    ) {
+      this.unlistenAbortSignal();
+      return Promise.reject(error);
     }
+
     this.pendingKeys.delete(key);
 
+    if (this.done) {
+      // Nothing left to abort!
+      this.unlistenAbortSignal();
+    }
+
     if (error) {
       Object.defineProperty(promise, "_error", { get: () => error });
-    } else {
-      Object.defineProperty(promise, "_data", { get: () => data });
+      this.subscriber?.(false);
+      return Promise.reject(error);
     }
 
+    Object.defineProperty(promise, "_data", { get: () => data });
     this.subscriber?.(false);
+    return data;
   }
 
   subscribe(fn: (aborted: boolean) => void) {
     this.subscriber = fn;
   }
 
   cancel() {
-    this.cancelled = true;
+    this.controller.abort();
     this.pendingKeys.forEach((v, k) => this.pendingKeys.delete(k));
     this.subscriber?.(true);
   }