feat(remix-server-runtime): RRR 1.1 - handleResourceRequest

integration/resource-routes-test.ts

@@ -65,6 +65,31 @@ test.describe("loader in an app", () => {
               });
             };
           `,
+          "app/routes/throw-error.jsx": js`
+            export let loader = () => {
+              throw new Error('Oh noes!')
+            }
+          `,
+          "app/routes/return-response.jsx": js`
+            export let loader = () => {
+              return new Response('Partial', { status: 207 });
+            }
+          `,
+          "app/routes/throw-response.jsx": js`
+            export let loader = () => {
+              throw new Response('Partial', { status: 207 });
+            }
+          `,
+          "app/routes/return-object.jsx": js`
+            export let loader = () => {
+              return { hello: 'world' };
+            }
+          `,
+          "app/routes/throw-object.jsx": js`
+            export let loader = () => {
+              throw { but: 'why' };
+            }
+          `,
         },
       })
     );
@@ -109,5 +134,56 @@ test.describe("loader in an app", () => {
       let res = await app.goto("/image.png");
       expect(res.status()).toBe(200);
     });
+
+    test("should handle errors thrown from resource routes", async ({
+      page,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      let res = await app.goto("/throw-error");
+      expect(res.status()).toBe(500);
+      // TODO: Is there any way to differentiate this from remix blowing up on
+      // the server?  In dev mode we include the error message in the response,
+      // but tests run against prod mode
+      expect(await res.text()).toEqual("Unexpected Server Error");
+    });
   }
+
+  test("should handle responses returned from resource routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    let res = await app.goto("/return-response");
+    expect(res.status()).toBe(207);
+    expect(await res.text()).toEqual("Partial");
+  });
+
+  test("should handle responses thrown from resource routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    let res = await app.goto("/throw-response");
+    expect(res.status()).toBe(207);
+    expect(await res.text()).toEqual("Partial");
+  });
+
+  test("should handle objects returned from resource routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    let res = await app.goto("/return-object");
+    expect(res.status()).toBe(200);
+    expect(await res.json()).toEqual({ hello: "world" });
+  });
+
+  test("should handle objects thrown from resource routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    let res = await app.goto("/throw-object");
+    expect(res.status()).toBe(500);
+    // TODO: Is there any way to differentiate this from remix blowing up on
+    // the server?  In dev mode we include the error message in the response,
+    // but tests run against prod mode
+    expect(await res.text()).toEqual("Unexpected Server Error");
+  });
 });

packages/remix-node/package.json

@@ -16,7 +16,7 @@
   "sideEffects": false,
   "dependencies": {
     "@remix-run/server-runtime": "1.7.1",
-    "@remix-run/web-fetch": "^4.1.3",
+    "@remix-run/web-fetch": "^4.3.0",
     "@remix-run/web-file": "^3.0.2",
     "@remix-run/web-stream": "^1.0.3",
     "@web3-storage/multipart-parser": "^1.0.0",

packages/remix-server-runtime/rollup.config.js

@@ -35,14 +35,17 @@ module.exports = function rollup() {
         return isBareModuleId(id);
       },
       input: `${sourceDir}/index.ts`,
-      treeshake: "smallest",
       output: {
         banner: createBanner(packageName, version),
         dir: outputDist,
         format: "cjs",
         preserveModules: true,
         exports: "named",
       },
+      treeshake: {
+        // Without this, we don't tree-shake the require('@remix-run/router') :/
+        moduleSideEffects: false,
+      },
       plugins: [
         replacePlugin,
         babel({
@@ -67,13 +70,16 @@ module.exports = function rollup() {
         return isBareModuleId(id);
       },
       input: `${sourceDir}/index.ts`,
-      treeshake: "smallest",
       output: {
         banner: createBanner(packageName, version),
         dir: `${outputDist}/esm`,
         format: "esm",
         preserveModules: true,
       },
+      treeshake: {
+        // Without this, we don't tree-shake the require('@remix-run/router') :/
+        moduleSideEffects: false,
+      },
       plugins: [
         replacePlugin,
         babel({

packages/remix-server-runtime/server.ts

@@ -10,6 +10,7 @@ import type { EntryContext } from "./entry";
 import { createEntryMatches, createEntryRouteModules } from "./entry";
 import { serializeError } from "./errors";
 import { getDocumentHeaders } from "./headers";
+import invariant from "./invariant";
 import { ServerMode, isServerMode } from "./mode";
 import type { RouteMatch } from "./routeMatching";
 import { matchServerRoutes } from "./routeMatching";
@@ -55,13 +56,13 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
       matches &&
       matches[matches.length - 1].route.module.default == null
     ) {
-      let responsePromise = handleResourceRequest({
+      response = await handleResourceRequest({
         request:
           // We need to clone the request here instead of the call to the new
           // handler otherwise the first handler will lock the body for the other.
           // Cloning here allows the new handler to be the stream reader and delegate
           // chunks back to this cloned request.
-          ENABLE_REACT_ROUTER && request.body ? request.clone() : request,
+          ENABLE_REACT_ROUTER ? request.clone() : request,
         loadContext,
         matches,
         serverMode,
@@ -74,24 +75,18 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
           createStaticHandlerDataRoutes(build.routes, loadContext)
         );
 
-        let [response, remixRouterResponse] = await Promise.all([
-          responsePromise,
-          handleResourceRequestRR(
-            serverMode,
-            staticHandler,
-            matches.slice(-1)[0].route.id,
-            request
-          ),
-        ]);
+        let remixRouterResponse = await handleResourceRequestRR(
+          serverMode,
+          staticHandler,
+          matches.slice(-1)[0].route.id,
+          request
+        );
 
         assertResponsesMatch(response, remixRouterResponse);
 
         console.log("Returning Remix Router Resource Request Response");
-        // response = remixRouterResponse;
-        responsePromise = Promise.resolve(response);
+        response = remixRouterResponse;
       }
-
-      response = await responsePromise;
     } else {
       response = await handleDocumentRequest({
         build,
@@ -568,22 +563,23 @@ async function handleResourceRequestRR(
   routeId: string,
   request: Request
 ) {
-  let response: Response | Error;
   try {
-    response = await staticHandler.queryRoute(request, routeId);
+    let response = await staticHandler.queryRoute(request, routeId);
+    // Remix should always be returning responses from loaders and actions
+    invariant(
+      response instanceof Response,
+      "Expected a Response to be returned from queryRoute"
+    );
+    return response;
   } catch (error) {
-    response = error as Error;
-  }
-
-  if (response instanceof Error) {
     if (serverMode !== ServerMode.Test) {
-      console.error(response);
+      console.error(error);
     }
 
     let message = "Unexpected Server Error";
 
     if (serverMode === ServerMode.Development) {
-      message += `\n\n${String(response)}`;
+      message += `\n\n${String(error)}`;
     }
 
     // Good grief folks, get your act together 😂!
@@ -594,8 +590,6 @@ async function handleResourceRequestRR(
       },
     });
   }
-
-  return response;
 }
 
 async function handleResourceRequest({

yarn.lock

@@ -2333,13 +2333,13 @@
     "@remix-run/web-stream" "^1.0.0"
     web-encoding "1.1.5"
 
-"@remix-run/web-fetch@^4.1.3":
-  version "4.1.3"
-  resolved "https://registry.npmjs.org/@remix-run/web-fetch/-/web-fetch-4.1.3.tgz"
-  integrity sha512-D3KXAEkzhR248mu7wCHReQrMrIo3Y9pDDa7TrlISnsOEvqkfWkJJF+PQWmOIKpOSHAhDg7TCb2tzvW8lc/MfHw==
+"@remix-run/web-fetch@^4.3.0":
+  version "4.3.0"
+  resolved "https://registry.yarnpkg.com/@remix-run/web-fetch/-/web-fetch-4.3.0.tgz#8990c58352ecefed2ea32e13a924c4cc0d7d7328"
+  integrity sha512-//VynoWVu/hocB0TWfoUZdg6Yy/320cO4r0dN/eZUSM5/rLUGEy+NqAykyY8GKHjYYZimw96kRMFa3aQbwmp8g==
   dependencies:
     "@remix-run/web-blob" "^3.0.4"
-    "@remix-run/web-form-data" "^3.0.2"
+    "@remix-run/web-form-data" "^3.0.3"
     "@remix-run/web-stream" "^1.0.3"
     "@web3-storage/multipart-parser" "^1.0.0"
     data-uri-to-buffer "^3.0.1"
@@ -2352,10 +2352,10 @@
   dependencies:
     "@remix-run/web-blob" "^3.0.3"
 
-"@remix-run/web-form-data@^3.0.2":
-  version "3.0.2"
-  resolved "https://registry.npmjs.org/@remix-run/web-form-data/-/web-form-data-3.0.2.tgz"
-  integrity sha512-F8tm3iB1sPxMpysK6Js7lV3gvLfTNKGmIW38t/e6dtPEB5L1WdbRG1cmLyhsonFc7rT1x1JKdz+2jCtoSdnIUw==
+"@remix-run/web-form-data@^3.0.3":
+  version "3.0.3"
+  resolved "https://registry.yarnpkg.com/@remix-run/web-form-data/-/web-form-data-3.0.3.tgz#f89a7f971aaf1084d2da87affbb7f4e01c32b8ce"
+  integrity sha512-wL4veBtVPazSpXfPMzrbmeV3IxuxCfcQYPerQ8BXRO5ahAEVw23tv7xS+yoX0XDO5j+vpRaSbhHJK1H5gF7eYQ==
   dependencies:
     web-encoding "1.1.5"