Better handling of deferred promises that resolve/reject with undefined (#10690)

Co-authored-by: Jacob Ebey <[email protected]>

Author: brophdawg11

.changeset/defer-resolve-undefined.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Better handling of deferred promises that resolve/reject with `undefined`

package.json

@@ -109,7 +109,7 @@
   },
   "filesize": {
     "packages/router/dist/router.umd.min.js": {
-      "none": "46.9 kB"
+      "none": "47.2 kB"
     },
     "packages/react-router/dist/react-router.production.min.js": {
       "none": "13.8 kB"

packages/router/__tests__/router-test.ts

@@ -14310,6 +14310,12 @@ describe("a router", () => {
                   ),
                 });
               }
+              if (new URL(request.url).searchParams.has("undefined")) {
+                return defer({
+                  critical: "loader",
+                  lazy: new Promise((r) => setTimeout(() => r(undefined), 10)),
+                });
+              }
               if (new URL(request.url).searchParams.has("status")) {
                 return defer(
                   {
@@ -15114,6 +15120,42 @@ describe("a router", () => {
           });
         });
 
+        it("should return rejected DeferredData on symbol for resolved undefined", async () => {
+          let context = (await query(
+            createRequest("/parent/deferred?undefined")
+          )) as StaticHandlerContext;
+          expect(context).toMatchObject({
+            loaderData: {
+              parent: "PARENT LOADER",
+              deferred: {
+                critical: "loader",
+                lazy: expect.trackedPromise(),
+              },
+            },
+            activeDeferreds: {
+              deferred: expect.deferredData(false),
+            },
+          });
+          await new Promise((r) => setTimeout(r, 10));
+          expect(context).toMatchObject({
+            loaderData: {
+              parent: "PARENT LOADER",
+              deferred: {
+                critical: "loader",
+                lazy: expect.trackedPromise(
+                  null,
+                  new Error(
+                    `Deferred data for key "lazy" resolved/rejected with \`undefined\`, you must resolve/reject with a value or \`null\`.`
+                  )
+                ),
+              },
+            },
+            activeDeferreds: {
+              deferred: expect.deferredData(true),
+            },
+          });
+        });
+
         it("should return DeferredData on symbol with status + headers", async () => {
           let context = (await query(
             createRequest("/parent/deferred?status")
@@ -16179,6 +16221,28 @@ describe("a router", () => {
           expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(true);
         });
 
+        it("should return rejected DeferredData on symbol for resolved undefined", async () => {
+          let result = await queryRoute(
+            createRequest("/parent/deferred?undefined")
+          );
+          expect(result).toMatchObject({
+            critical: "loader",
+            lazy: expect.trackedPromise(),
+          });
+          expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(false);
+          await new Promise((r) => setTimeout(r, 10));
+          expect(result).toMatchObject({
+            critical: "loader",
+            lazy: expect.trackedPromise(
+              null,
+              new Error(
+                `Deferred data for key "lazy" resolved/rejected with \`undefined\`, you must resolve/reject with a value or \`null\`.`
+              )
+            ),
+          });
+          expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(true);
+        });
+
         it("should return DeferredData on symbol with status + headers", async () => {
           let result = await queryRoute(
             createRequest("/parent/deferred?status")

packages/router/utils.ts

@@ -1317,7 +1317,7 @@ export class DeferredData {
     // We store a little wrapper promise that will be extended with
     // _data/_error props upon resolve/reject
     let promise: TrackedPromise = Promise.race([value, this.abortPromise]).then(
-      (data) => this.onSettle(promise, key, null, data as unknown),
+      (data) => this.onSettle(promise, key, undefined, data as unknown),
       (error) => this.onSettle(promise, key, error as unknown)
     );
 
@@ -1351,7 +1351,19 @@ export class DeferredData {
       this.unlistenAbortSignal();
     }
 
-    if (error) {
+    // If the promise was resolved/rejected with undefined, we'll throw an error as you
+    // should always resolve with a value or null
+    if (error === undefined && data === undefined) {
+      let undefinedError = new Error(
+        `Deferred data for key "${key}" resolved/rejected with \`undefined\`, ` +
+          `you must resolve/reject with a value or \`null\`.`
+      );
+      Object.defineProperty(promise, "_error", { get: () => undefinedError });
+      this.emit(false, key);
+      return Promise.reject(undefinedError);
+    }
+
+    if (data === undefined) {
       Object.defineProperty(promise, "_error", { get: () => error });
       this.emit(false, key);
       return Promise.reject(error);