feat(remix-server-runtime): RRR 1.2 - handleDataRequest (#4359)

chore: updated router to latest chore: improve test reliability Co-authored-by: Matt Brophy <matt@brophy.org>
remix-run · Oct 17, 2022 · 237a289 · 237a289
1 parent eb38258
commit 237a289
Show file tree

Hide file tree

Showing 17 changed files with 557 additions and 243 deletions.
diff --git a/.changeset/five-mayflies-train.md b/.changeset/five-mayflies-train.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+add handleDataRequest implementation based on @remix-run/router behind experimental build flag
diff --git a/.changeset/stale-ravens-wink.md b/.changeset/stale-ravens-wink.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/node": patch
+---
+
+update @remix-run/web-fetch to 4.3.1
diff --git a/integration/abort-signal-test.ts b/integration/abort-signal-test.ts
@@ -15,14 +15,12 @@ test.beforeAll(async () => {
         import { useActionData, useLoaderData, Form } from "@remix-run/react";
 
         export async function action ({ request }) {
-          console.log('action request.signal', request.signal)
           // New event loop causes express request to close
           await new Promise(r => setTimeout(r, 0));
           return json({ aborted: request.signal.aborted });
         }
 
         export function loader({ request }) {
-          console.log('loader request.signal', request.signal)
           return json({ aborted: request.signal.aborted });
         }
 

diff --git a/integration/catch-boundary-test.ts b/integration/catch-boundary-test.ts
@@ -46,7 +46,7 @@ test.describe("CatchBoundary", () => {
               <html>
                 <head />
                 <body>
-                  <div>${ROOT_BOUNDARY_TEXT}</div>
+                  <div id="root-boundary">${ROOT_BOUNDARY_TEXT}</div>
                   <Scripts />
                 </body>
               </html>
@@ -82,7 +82,7 @@ test.describe("CatchBoundary", () => {
         "app/routes/fetcher-boundary.jsx": js`
           import { useFetcher } from "@remix-run/react";
           export function CatchBoundary() {
-            return <p>${OWN_BOUNDARY_TEXT}</p>
+            return <p id="fetcher-boundary">${OWN_BOUNDARY_TEXT}</p>
           }
           export default function() {
             let fetcher = useFetcher();
@@ -118,7 +118,7 @@ test.describe("CatchBoundary", () => {
             throw new Response("", { status: 401 })
           }
           export function CatchBoundary() {
-            return <p>${OWN_BOUNDARY_TEXT}</p>
+            return <p id="action-boundary">${OWN_BOUNDARY_TEXT}</p>
           }
           export default function Index() {
             return (
@@ -149,7 +149,7 @@ test.describe("CatchBoundary", () => {
 
         [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
           export function CatchBoundary() {
-            return <div>${OWN_BOUNDARY_TEXT}</div>
+            return <div id="boundary-no-loader-or-action">${OWN_BOUNDARY_TEXT}</div>
           }
           export default function Index() {
             return <div/>
@@ -167,7 +167,7 @@ test.describe("CatchBoundary", () => {
             throw new Response("", { status: 401 })
           }
           export function CatchBoundary() {
-            return <div>${OWN_BOUNDARY_TEXT}</div>
+            return <div id="boundary-loader">${OWN_BOUNDARY_TEXT}</div>
           }
           export default function Index() {
             return <div/>
@@ -348,7 +348,7 @@ test.describe("CatchBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    expect(await app.getHtml()).toMatch(ROOT_BOUNDARY_TEXT);
+    await page.waitForSelector("#root-boundary");
   });
 
   test("renders own boundary in document POST without action requests", async () => {
@@ -365,7 +365,7 @@ test.describe("CatchBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickSubmitButton(HAS_BOUNDARY_NO_LOADER_OR_ACTION);
-    expect(await app.getHtml()).toMatch(OWN_BOUNDARY_TEXT);
+    await page.waitForSelector("#boundary-no-loader-or-action");
   });
 
   test("renders own boundary in fetcher action submission without action from other routes", async ({
@@ -374,7 +374,7 @@ test.describe("CatchBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/fetcher-boundary");
     await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    expect(await app.getHtml()).toMatch(OWN_BOUNDARY_TEXT);
+    await page.waitForSelector("#fetcher-boundary");
   });
 
   test("renders root boundary in fetcher action submission without action from other routes", async ({
@@ -383,7 +383,7 @@ test.describe("CatchBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/fetcher-no-boundary");
     await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    expect(await app.getHtml()).toMatch(ROOT_BOUNDARY_TEXT);
+    await page.waitForSelector("#root-boundary");
   });
 
   test("uses correct catch boundary on server action errors", async ({

diff --git a/integration/deterministic-build-output-test.ts b/integration/deterministic-build-output-test.ts
@@ -38,9 +38,11 @@ test("builds deterministically under different paths", async () => {
   let files1 = await globby(["build/index.js", "public/build/**/*.js"], {
     cwd: dir1,
   });
+  files1 = files1.sort();
   let files2 = await globby(["build/index.js", "public/build/**/*.js"], {
     cwd: dir2,
   });
+  files2 = files2.sort();
 
   expect(files1.length).toBeGreaterThan(0);
   expect(files1).toEqual(files2);

diff --git a/integration/form-test.ts b/integration/form-test.ts
@@ -419,7 +419,7 @@ test.describe("Forms", () => {
         page.waitForNavigation(),
         app.clickSubmitButton("/get-submission", { wait: false }),
       ]);
-      expect(await app.getHtml("pre")).toMatch(CHEESESTEAK);
+      await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
     });
   });
 
@@ -429,7 +429,7 @@ test.describe("Forms", () => {
       // this indirectly tests that clicking SVG children in buttons works
       await app.goto("/get-submission");
       await app.clickSubmitButton("/get-submission");
-      expect(await app.getHtml("pre")).toMatch(CHEESESTEAK);
+      await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
     });
   });
 
@@ -440,7 +440,7 @@ test.describe("Forms", () => {
     await app.goto("/get-submission");
     await app.clickElement(`#${FORM_WITH_ACTION_INPUT} button`);
     await page.waitForLoadState("load");
-    expect(await app.getHtml("pre")).toMatch(EAT);
+    await page.waitForSelector(`pre:has-text("${EAT}")`);
   });
 
   test("posts to a loader with button data with click", async ({ page }) => {
@@ -450,7 +450,7 @@ test.describe("Forms", () => {
       page.waitForNavigation(),
       app.clickElement("#buttonWithValue"),
     ]);
-    expect(await app.getHtml("pre")).toMatch(LAKSA);
+    await page.waitForSelector(`pre:has-text("${LAKSA}")`);
   });
 
   test("posts to a loader with button data with keyboard", async ({ page }) => {
@@ -459,23 +459,22 @@ test.describe("Forms", () => {
     await page.focus(`#${KEYBOARD_INPUT}`);
     await page.keyboard.press("Enter");
     await page.waitForLoadState("networkidle");
-    expect(await app.getHtml("pre")).toMatch(LAKSA);
+    await page.waitForSelector(`pre:has-text("${LAKSA}")`);
   });
 
   test("posts with the correct checkbox data", async ({ page }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/get-submission");
     await app.clickElement(`#${CHECKBOX_BUTTON}`);
-    expect(await app.getHtml("pre")).toBe(
-      `<pre>LUNCH=CHEESESTEAK&amp;LUNCH=LAKSA</pre>`
-    );
+    await page.waitForSelector(`pre:has-text("${LAKSA}")`);
+    await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
   });
 
   test("posts button data from outside the form", async ({ page }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/get-submission");
     await app.clickElement(`#${ORPHAN_BUTTON}`);
-    expect(await app.getHtml("pre")).toMatch(SQUID_INK_HOTDOG);
+    await page.waitForSelector(`pre:has-text("${SQUID_INK_HOTDOG}")`);
   });
 
   test("when clicking on a submit button as a descendant of an element that stops propagation on click, still passes the clicked submit button's `name` and `value` props to the request payload", async ({

diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts
@@ -42,7 +42,7 @@ export async function createFixture(init: FixtureInit) {
     let url = new URL(href, "test://test");
     let request = new Request(url.toString(), {
       ...init,
-      signal: new AbortController().signal,
+      signal: init?.signal || new AbortController().signal,
     });
     return handler(request);
   };
@@ -52,6 +52,8 @@ export async function createFixture(init: FixtureInit) {
     routeId: string,
     init?: RequestInit
   ) => {
+    init = init || {};
+    init.signal = init.signal || new AbortController().signal;
     let url = new URL(href, "test://test");
     url.searchParams.set("_data", routeId);
     let request = new Request(url.toString(), init);

diff --git a/integration/redirects-test.ts b/integration/redirects-test.ts
@@ -16,10 +16,16 @@ test.describe("redirects", () => {
 
           if (typeof global.actionCount === "undefined") {
             global.actionCount = 0;
+            global.actionRequests = new Set();
           }
 
-          export async function loader({ request }) {
-            return { count: ++global.actionCount };
+          export async function loader({ request, context }) {
+            let count = global.actionCount;
+            if (!global.actionRequests.has(context)) {
+              global.actionRequests.add(context);
+              count = ++global.actionCount;
+            }
+            return { count };
           };
 
           export default function Parent() {
@@ -75,12 +81,18 @@ test.describe("redirects", () => {
 
           if (typeof global.loaderCount === "undefined") {
             global.loaderCount = 0;
+            global.loaderRequests = new Set();
           }
 
-          export async function loader({ request }) {
+          export async function loader({ request, context }) {
             const cookieHeader = request.headers.get("Cookie");
             const { value } = (await session.parse(cookieHeader)) || {};
-            return { count: ++global.loaderCount, value };
+            let count = global.loaderCount;
+            if (!global.loaderRequests.has(context)) {
+              global.loaderRequests.add(context);
+              count = ++global.loaderCount;
+            }
+            return { count, value };
           };
 
           export default function Parent() {
@@ -150,7 +162,9 @@ test.describe("redirects", () => {
     expect(await app.getHtml("#count")).toMatch("1");
     // Submitting this form will trigger an action -> redirect -> redirect
     // and we need to ensure that the parent loader is called on both redirects
-    await app.clickElement('button[type="submit"]');
+    await app.waitForNetworkAfter(() =>
+      app.clickElement('button[type="submit"]')
+    );
     expect(await app.getHtml("#app")).toMatch("Page 2");
     // Loader called twice
     expect(await app.getHtml("#count")).toMatch("3");
@@ -165,7 +179,9 @@ test.describe("redirects", () => {
     // Clicking this link will trigger a normalRedirect -> normalRedirect with
     // a cookie set on the first one, and we need to ensure that the parent
     // loader is called on both redirects
-    await app.clickElement('a[href="/loader/redirect"]');
+    await app.waitForNetworkAfter(() =>
+      app.clickElement('a[href="/loader/redirect"]')
+    );
     expect(await app.getHtml("#app")).toMatch("Page 2");
     expect(await app.getHtml("#app")).toMatch("cookie-value");
     // Loader called twice

diff --git a/integration/transition-test.ts b/integration/transition-test.ts
@@ -156,9 +156,17 @@ test.describe("rendering", () => {
         "app/routes/parent.jsx": js`
           import { Outlet, useLoaderData } from "@remix-run/react";
 
-          let count = 0;
-          export const loader = async ({ request }) => {
-            return { count: ++count };
+          if (!global.counts) {
+            global.count = 0;
+            global.counts = new Set();
+          }
+          export const loader = async ({ request, context }) => {
+            let count = global.count;
+            if (!global.counts.has(context)) {
+              counts.add(context);
+              count = ++global.count;
+            }
+            return { count };
           };
 
           export default function Parent() {
@@ -239,8 +247,8 @@ test.describe("rendering", () => {
     expect(new URL(page.url()).pathname).toBe(`/${REDIRECT_TARGET}`);
 
     expect(
-      responses.map((res) => new URL(res.url()).searchParams.get("_data"))
-    ).toEqual([`routes/${REDIRECT}`, `routes/${PAGE}`, `routes/${PAGE}/index`]);
+      responses.map((res) => new URL(res.url()).searchParams.get("_data")).sort()
+    ).toEqual([`routes/${REDIRECT}`, `routes/${PAGE}`, `routes/${PAGE}/index`].sort());
 
     let html = await app.getHtml("main");
     expect(html).toMatch(PAGE_TEXT);

diff --git a/packages/remix-node/package.json b/packages/remix-node/package.json
@@ -16,7 +16,7 @@
   "sideEffects": false,
   "dependencies": {
     "@remix-run/server-runtime": "1.7.2",
-    "@remix-run/web-fetch": "^4.3.0",
+    "@remix-run/web-fetch": "^4.3.1",
     "@remix-run/web-file": "^3.0.2",
     "@remix-run/web-stream": "^1.0.3",
     "@web3-storage/multipart-parser": "^1.0.0",

diff --git a/packages/remix-server-runtime/__tests__/formData-test.ts b/packages/remix-server-runtime/__tests__/formData-test.ts
@@ -161,8 +161,7 @@ describe("parseMultipartFormData", () => {
 
       let error: Error;
       try {
-        let formData = await parseMultipartFormData(req, async () => undefined);
-        console.log(formData);
+        await parseMultipartFormData(req, async () => undefined);
         throw new Error("should have thrown");
       } catch (err) {
         error = err;