From ed18fb82cee8657e2e5c6b30347a8713a4ca9313 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jul 2024 17:27:56 -0400 Subject: [PATCH 01/13] Remove single fetch response stub, add headers --- .changeset/polite-pumas-visit.md | 5 + integration/single-fetch-test.ts | 1369 ++--------------- packages/remix-server-runtime/server.ts | 100 +- packages/remix-server-runtime/single-fetch.ts | 283 +--- 4 files changed, 222 insertions(+), 1535 deletions(-) create mode 100644 .changeset/polite-pumas-visit.md diff --git a/.changeset/polite-pumas-visit.md b/.changeset/polite-pumas-visit.md new file mode 100644 index 00000000000..1774910e3fb --- /dev/null +++ b/.changeset/polite-pumas-visit.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Single Fetch: Remove `responseStub` in favor of `headers` diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 67d2189e205..b8f3fdbbce0 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -15,6 +15,16 @@ const files = { "app/root.tsx": js` import { Form, Link, Links, Meta, Outlet, Scripts } from "@remix-run/react"; + export function headers ({ actionHeaders, loaderHeaders, errorHeaders }) { + if (errorHeaders) { + return errorHeaders; + } else if ([...actionHeaders].length > 0) { + return actionHeaders; + } else { + return loaderHeaders; + } + } + export function loader() { return { message: "ROOT", @@ -152,8 +162,6 @@ test.describe("single-fetch", () => { ServerMode.Development ); - console.error = () => {}; - let res = await fixture.requestSingleFetchData("/data.data?error=true"); expect(res.data).toEqual({ root: { @@ -335,23 +343,19 @@ test.describe("single-fetch", () => { "app/routes/action.tsx": js` import { Form, Link, useActionData, useLoaderData, useNavigation } from '@remix-run/react'; - export async function action({ request, response }) { + export async function action({ request }) { let fd = await request.formData(); if (fd.get('throw') === "5xx") { - response.status = 500; - throw new Error("Thrown 500"); + throw new Response("Thrown 500", { status: 500 }); } if (fd.get('throw') === "4xx") { - response.status = 400; - throw new Error("Thrown 400"); + throw new Response("Thrown 400", { status: 400 }); } if (fd.get('return') === "5xx") { - response.status = 500; - return "Returned 500"; + return new Response("Returned 500", { status: 500 }); } if (fd.get('return') === "4xx") { - response.status = 400; - return "Returned 400"; + return new Response("Returned 400", { status: 400 }); } return null; } @@ -437,22 +441,28 @@ test.describe("single-fetch", () => { files: { ...files, "app/routes/headers.tsx": js` - export function action({ request, response }) { + export function headers ({ actionHeaders, loaderHeaders, errorHeaders }) { + if (errorHeaders) { + return errorHeaders; + } else if ([...actionHeaders].length > 0) { + return actionHeaders; + } else { + return loaderHeaders; + } + } + + export function action({ request }) { if (new URL(request.url).searchParams.has("error")) { - response.headers.set("x-action-error", "true"); - throw response; + throw new Response(null, { headers: { "x-action-error": "true" } }); } - response.headers.set("x-action", "true"); - return null; + return new Response(null, { headers: { "x-action": "true" } }); } - export function loader({ request, response }) { + export function loader({ request }) { if (new URL(request.url).searchParams.has("error")) { - response.headers.set("x-loader-error", "true"); - throw response; + throw new Response(null, { headers: { "x-loader-error": "true" } }); } - response.headers.set("x-loader", "true"); - return null; + return new Response(null, { headers: { "x-loader": "true" } }); } export default function Comp() { @@ -501,166 +511,7 @@ test.describe("single-fetch", () => { expect(dataResponse.headers.get("x-action-error")).toEqual("true"); }); - test("merges headers from nested routes", async () => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/a.tsx": js` - export function loader({ request, response }) { - response.headers.set('x-one', 'a set'); - response.headers.append('x-one', 'a append'); - response.headers.set('x-two', 'a set'); - response.headers.append('x-three', 'a append'); - response.headers.set('x-four', 'a set'); - return null; - } - - export default function Comp() { - return null; - } - `, - "app/routes/a.b.tsx": js` - export function loader({ request, response }) { - response.headers.set('x-one', 'b set'); - response.headers.append('x-one', 'b append'); - response.headers.set('x-two', 'b set'); - response.headers.append('x-three', 'b append'); - response.headers.delete('x-four'); - return null; - } - - export default function Comp() { - return null; - } - `, - "app/routes/a.b.c.tsx": js` - export function action({ request, response }) { - response.headers.set('x-one', 'c action set'); - response.headers.append('x-one', 'c action append'); - response.headers.set('x-two', 'c action set'); - response.headers.append('x-three', 'c action append'); - response.headers.set('x-four', 'c action set'); - return null; - } - - export function loader({ request, response }) { - response.headers.set('x-one', 'c set'); - response.headers.append('x-one', 'c append'); - response.headers.set('x-two', 'c set'); - response.headers.append('x-three', 'c append'); - return null; - } - - export default function Comp() { - return null; - } - `, - }, - }); - - // x-one uses both set and append - // x-two only uses set - // x-three only uses append - // x-four deletes - let res: Awaited< - ReturnType< - typeof fixture.requestDocument | typeof fixture.requestSingleFetchData - > - >; - res = await fixture.requestDocument("/a"); - expect(res.headers.get("x-one")).toEqual("a set, a append"); - expect(res.headers.get("x-two")).toEqual("a set"); - expect(res.headers.get("x-three")).toEqual("a append"); - expect(res.headers.get("x-four")).toEqual("a set"); - - res = await fixture.requestSingleFetchData("/a.data"); - expect(res.headers.get("x-one")).toEqual("a set, a append"); - expect(res.headers.get("x-two")).toEqual("a set"); - expect(res.headers.get("x-three")).toEqual("a append"); - expect(res.headers.get("x-four")).toEqual("a set"); - - res = await fixture.requestDocument("/a/b"); - expect(res.headers.get("x-one")).toEqual("b set, b append"); - expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("a append, b append"); - expect(res.headers.get("x-four")).toEqual(null); - - res = await fixture.requestSingleFetchData("/a/b.data"); - expect(res.headers.get("x-one")).toEqual("b set, b append"); - expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("a append, b append"); - expect(res.headers.get("x-four")).toEqual(null); - - res = await fixture.requestDocument("/a/b/c"); - expect(res.headers.get("x-one")).toEqual("c set, c append"); - expect(res.headers.get("x-two")).toEqual("c set"); - expect(res.headers.get("x-three")).toEqual("a append, b append, c append"); - expect(res.headers.get("x-four")).toEqual(null); - - res = await fixture.requestSingleFetchData("/a/b/c.data"); - expect(res.headers.get("x-one")).toEqual("c set, c append"); - expect(res.headers.get("x-two")).toEqual("c set"); - expect(res.headers.get("x-three")).toEqual("a append, b append, c append"); - expect(res.headers.get("x-four")).toEqual(null); - - // Fine-grained revalidation - res = await fixture.requestDocument("/a/b/c.data?_routes=routes%2Fa"); - expect(res.headers.get("x-one")).toEqual("a set, a append"); - expect(res.headers.get("x-two")).toEqual("a set"); - expect(res.headers.get("x-three")).toEqual("a append"); - expect(res.headers.get("x-four")).toEqual("a set"); - - res = await fixture.requestDocument( - "/a/b.data?_routes=routes%2Fa,routes%2Fa.b" - ); - expect(res.headers.get("x-one")).toEqual("b set, b append"); - expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("a append, b append"); - expect(res.headers.get("x-four")).toEqual(null); - - res = await fixture.requestDocument("/a/b/c.data?_routes=routes%2Fa.b.c"); - expect(res.headers.get("x-one")).toEqual("c set, c append"); - expect(res.headers.get("x-two")).toEqual("c set"); - expect(res.headers.get("x-three")).toEqual("c append"); - expect(res.headers.get("x-four")).toEqual(null); - - res = await fixture.requestDocument( - "/a/b/c.data?_routes=routes%2Fa,routes%2Fa.b.c" - ); - expect(res.headers.get("x-one")).toEqual("c set, c append"); - expect(res.headers.get("x-two")).toEqual("c set"); - expect(res.headers.get("x-three")).toEqual("a append, c append"); - expect(res.headers.get("x-four")).toEqual("a set"); - - // Action only - single fetch request - res = await fixture.requestSingleFetchData("/a/b/c.data", { - method: "post", - body: null, - }); - expect(res.headers.get("x-one")).toEqual("c action set, c action append"); - expect(res.headers.get("x-two")).toEqual("c action set"); - expect(res.headers.get("x-three")).toEqual("c action append"); - expect(res.headers.get("x-four")).toEqual("c action set"); - - // Actions and Loaders - Document request - res = await fixture.requestDocument("/a/b/c", { - method: "post", - body: null, - }); - expect(res.headers.get("x-one")).toEqual("c set, c append"); - expect(res.headers.get("x-two")).toEqual("c set"); - expect(res.headers.get("x-three")).toEqual( - "c action append, a append, b append, c append" - ); - expect(res.headers.get("x-four")).toEqual(null); - }); - - test("merges status codes from nested routes", async () => { + test("merges headers from nested routes when raw Responses are returned", async () => { let fixture = await createFixture({ config: { future: { @@ -670,115 +521,14 @@ test.describe("single-fetch", () => { files: { ...files, "app/routes/a.tsx": js` - export function loader({ request, response }) { - if (new URL(request.url).searchParams.has("error")) { - response.status = 401 - } else { - response.status = 201 - } - return null; - } - - export default function Comp() { - return null; - } - `, - "app/routes/a.b.tsx": js` - export function loader({ request, response }) { - response.status = 202 - return null; - } - - export default function Comp() { - return null; - } - `, - "app/routes/a.b.c.tsx": js` - export function action({ request, response }) { - response.status = 206 - return null; - } - - export function loader({ request, response }) { - response.status = 203 - return null; + export function headers({ loaderHeaders }) { + return loaderHeaders; } - export default function Comp() { - return null; - } - `, - }, - }); - - // Loaders - let res: Awaited< - ReturnType< - typeof fixture.requestDocument | typeof fixture.requestSingleFetchData - > - >; - res = await fixture.requestDocument("/a"); - expect(res.status).toEqual(201); - - res = await fixture.requestSingleFetchData("/a.data"); - expect(res.status).toEqual(201); - - res = await fixture.requestDocument("/a/b"); - expect(res.status).toEqual(202); - - res = await fixture.requestSingleFetchData("/a/b.data"); - expect(res.status).toEqual(202); - - res = await fixture.requestDocument("/a/b/c"); - expect(res.status).toEqual(203); - - res = await fixture.requestSingleFetchData("/a/b/c.data"); - expect(res.status).toEqual(203); - - // Errors - res = await fixture.requestDocument("/a?error"); - expect(res.status).toEqual(401); - - res = await fixture.requestSingleFetchData("/a.data?error"); - expect(res.status).toEqual(401); - - res = await fixture.requestDocument("/a/b?error"); - expect(res.status).toEqual(401); - - res = await fixture.requestSingleFetchData("/a/b.data?error"); - expect(res.status).toEqual(401); - - // Actions - res = await fixture.requestDocument("/a/b/c", { - method: "post", - body: null, - }); - expect(res.status).toEqual(206); - - res = await fixture.requestSingleFetchData("/a/b/c.data", { - method: "post", - body: null, - }); - expect(res.status).toEqual(206); - }); - - test("merges headers from nested routes when raw Responses are returned", async () => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/a.tsx": js` - export function loader({ request}) { + export function loader({ request }) { let headers = new Headers(); headers.set('x-one', 'a set'); - headers.append('x-one', 'a append'); headers.set('x-two', 'a set'); - headers.append('x-three', 'a append'); - headers.set('x-four', 'a set'); return new Response(null, { headers }); } @@ -787,23 +537,29 @@ test.describe("single-fetch", () => { } `, "app/routes/a.b.tsx": js` - export function action({ request, response }) { + export function headers({ actionHeaders, loaderHeaders, parentHeaders }) { + let h = new Headers(); + for (let [name, value] of parentHeaders) { + h.set(name, value); + } + for (let [name, value] of loaderHeaders) { + h.set(name, value); + } + for (let [name, value] of actionHeaders) { + h.set(name, value); + } + return h; + } + + export function action({ request }) { let headers = new Headers(); - headers.set('x-one', 'b action set'); - headers.append('x-one', 'b action append'); headers.set('x-two', 'b action set'); - headers.append('x-three', 'b action append'); - headers.set('x-four', 'b action set'); return new Response(null, { headers }); } - export function loader({ request, response }) { + export function loader({ request }) { let headers = new Headers(); - headers.set('x-one', 'b set'); - headers.append('x-one', 'b append'); headers.set('x-two', 'b set'); - headers.append('x-three', 'b append'); - headers.delete('x-four'); return new Response(null, { headers }); } @@ -824,48 +580,36 @@ test.describe("single-fetch", () => { > >; res = await fixture.requestDocument("/a"); - expect(res.headers.get("x-one")).toEqual("a set, a append"); + expect(res.headers.get("x-one")).toEqual("a set"); expect(res.headers.get("x-two")).toEqual("a set"); - expect(res.headers.get("x-three")).toEqual("a append"); - expect(res.headers.get("x-four")).toEqual("a set"); res = await fixture.requestSingleFetchData("/a.data"); - expect(res.headers.get("x-one")).toEqual("a set, a append"); + expect(res.headers.get("x-one")).toEqual("a set"); expect(res.headers.get("x-two")).toEqual("a set"); - expect(res.headers.get("x-three")).toEqual("a append"); - expect(res.headers.get("x-four")).toEqual("a set"); res = await fixture.requestDocument("/a/b"); - expect(res.headers.get("x-one")).toEqual("b set, b append"); + expect(res.headers.get("x-one")).toEqual("a set"); expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("b append"); // Blows away "a append" - expect(res.headers.get("x-four")).toEqual("a set"); res = await fixture.requestSingleFetchData("/a/b.data"); - expect(res.headers.get("x-one")).toEqual("b set, b append"); + expect(res.headers.get("x-one")).toEqual("a set"); expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("b append"); // Blows away "a append" - expect(res.headers.get("x-four")).toEqual("a set"); // Action only - single fetch request res = await fixture.requestSingleFetchData("/a/b.data", { method: "post", body: null, }); - expect(res.headers.get("x-one")).toEqual("b action set, b action append"); + expect(res.headers.get("x-one")).toEqual(null); expect(res.headers.get("x-two")).toEqual("b action set"); - expect(res.headers.get("x-three")).toEqual("b action append"); - expect(res.headers.get("x-four")).toEqual("b action set"); // Actions and Loaders - Document request res = await fixture.requestDocument("/a/b", { method: "post", body: null, }); - expect(res.headers.get("x-one")).toEqual("b set, b append"); - expect(res.headers.get("x-two")).toEqual("b set"); - expect(res.headers.get("x-three")).toEqual("b append"); // Blows away prior appends - expect(res.headers.get("x-four")).toEqual("a set"); // Can't delete via Response + expect(res.headers.get("x-one")).toEqual("a set"); + expect(res.headers.get("x-two")).toEqual("b action set"); }); test("merges status codes from nested routes when raw Responses are used", async () => { @@ -878,9 +622,9 @@ test.describe("single-fetch", () => { files: { ...files, "app/routes/a.tsx": js` - export function loader({ request, response }) { + export function loader({ request }) { if (new URL(request.url).searchParams.has("error")) { - return new Response(null, { status: 401 }); + throw new Response(null, { status: 401 }); } else { return new Response(null, { status: 201 }); } @@ -891,7 +635,7 @@ test.describe("single-fetch", () => { } `, "app/routes/a.b.tsx": js` - export function loader({ request, response }) { + export function loader({ request }) { return new Response(null, { status: 202 }); } @@ -900,11 +644,11 @@ test.describe("single-fetch", () => { } `, "app/routes/a.b.c.tsx": js` - export function action({ request, response }) { + export function action({ request }) { return new Response(null, { status: 206 }); } - export function loader({ request, response }) { + export function loader({ request }) { return new Response(null, { status: 203 }); } @@ -915,6 +659,8 @@ test.describe("single-fetch", () => { }, }); + console.error = () => {}; + // Loaders let res: Awaited< ReturnType< @@ -966,9 +712,7 @@ test.describe("single-fetch", () => { expect(res.status).toEqual(206); }); - test("processes thrown loader redirects via responseStub", async ({ - page, - }) => { + test("processes thrown loader redirects via Response", async ({ page }) => { let fixture = await createFixture({ config: { future: { @@ -979,10 +723,8 @@ test.describe("single-fetch", () => { ...files, "app/routes/data.tsx": js` import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - throw response; + export function loader() { + throw redirect('/target'); } export default function Component() { return null @@ -1022,51 +764,7 @@ test.describe("single-fetch", () => { expect(await app.getHtml("#target")).toContain("Target"); }); - test("processes thrown loader redirects via responseStub (resource route)", async ({ - page, - }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - throw response; - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - - console.error = () => {}; - - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned loader redirects via responseStub", async ({ - page, - }) => { + test("processes returned loader redirects via Response", async ({ page }) => { let fixture = await createFixture({ config: { future: { @@ -1077,10 +775,8 @@ test.describe("single-fetch", () => { ...files, "app/routes/data.tsx": js` import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return response + export function loader() { + return redirect('/target'); } export default function Component() { return null @@ -1093,12 +789,22 @@ test.describe("single-fetch", () => { `, }, }); - let res = await fixture.requestDocument("/data"); expect(res.status).toBe(302); expect(res.headers.get("Location")).toBe("/target"); expect(await res.text()).toBe(""); + let { status, data } = await fixture.requestSingleFetchData("/data.data"); + expect(data).toEqual({ + [SingleFetchRedirectSymbol]: { + status: 302, + redirect: "/target", + reload: false, + revalidate: false, + }, + }); + expect(status).toBe(202); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); @@ -1107,554 +813,6 @@ test.describe("single-fetch", () => { expect(await app.getHtml("#target")).toContain("Target"); }); - test("processes loader redirects via responseStub when null is returned", async ({ - page, - }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return null - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let { status, data } = await fixture.requestSingleFetchData("/data.data"); - expect(data).toEqual({ - [SingleFetchRedirectSymbol]: { - status: 302, - redirect: "/target", - reload: false, - revalidate: false, - }, - }); - expect(status).toBe(202); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned loader redirects via responseStub (resource route)", async ({ - page, - }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return response; - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes loader redirects via responseStub when null is returned (resource route)", async ({ - page, - }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader({ request, response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes thrown loader redirects via Response", async ({ page }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader() { - throw redirect('/target'); - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - - console.error = () => {}; - - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let { status, data } = await fixture.requestSingleFetchData("/data.data"); - expect(data).toEqual({ - [SingleFetchRedirectSymbol]: { - status: 302, - redirect: "/target", - reload: false, - revalidate: false, - }, - }); - expect(status).toBe(202); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned loader redirects via Response", async ({ page }) => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function loader() { - return redirect('/target'); - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }); - let res = await fixture.requestDocument("/data"); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let { status, data } = await fixture.requestSingleFetchData("/data.data"); - expect(data).toEqual({ - [SingleFetchRedirectSymbol]: { - status: 302, - redirect: "/target", - reload: false, - revalidate: false, - }, - }); - expect(status).toBe(202); - - let appFixture = await createAppFixture(fixture); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes thrown action redirects via responseStub", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - throw response; - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - console.error = () => {}; - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let { status, data } = await fixture.requestSingleFetchData("/data.data", { - method: "post", - body: null, - }); - expect(data).toEqual({ - status: 302, - redirect: "/target", - reload: false, - revalidate: false, - }); - expect(status).toBe(202); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes thrown action redirects via responseStub (resource route)", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - throw response; - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - console.error = () => {}; - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned action redirects via responseStub", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return response - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes action redirects via responseStub when null is returned", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return null - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let { status, data } = await fixture.requestSingleFetchData("/data.data", { - method: "post", - body: null, - }); - expect(data).toEqual({ - status: 302, - redirect: "/target", - reload: false, - revalidate: false, - }); - expect(status).toBe(202); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned action redirects via responseStub (resource route)", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return response - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - - test("processes returned action redirects via responseStub when null is returned (resource route)", async ({ - page, - }) => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from '@remix-run/node'; - export function action({ response }) { - response.status = 302; - response.headers.set('Location', '/target'); - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, - }, - ServerMode.Development - ); - - let res = await fixture.requestDocument("/data", { - method: "post", - body: null, - }); - expect(res.status).toBe(302); - expect(res.headers.get("Location")).toBe("/target"); - expect(await res.text()).toBe(""); - - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickSubmitButton("/data"); - await page.waitForSelector("#target"); - expect(await app.getHtml("#target")).toContain("Target"); - }); - test("processes thrown action redirects via Response", async ({ page }) => { let fixture = await createFixture( { @@ -1771,6 +929,84 @@ test.describe("single-fetch", () => { expect(await app.getHtml("#target")).toContain("Target"); }); + test("processes thrown loader redirects (resource route)", async ({ + page, + }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/data.tsx": js` + import { redirect } from '@remix-run/node'; + export function loader({ request }) { + throw redirect('/target'); + } + `, + "app/routes/target.tsx": js` + export default function Component() { + return

Target

+ } + `, + }, + }); + + console.error = () => {}; + + let res = await fixture.requestDocument("/data"); + expect(res.status).toBe(302); + expect(res.headers.get("Location")).toBe("/target"); + expect(await res.text()).toBe(""); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickLink("/data"); + await page.waitForSelector("#target"); + expect(await app.getHtml("#target")).toContain("Target"); + }); + + test("processes returned loader redirects (resource route)", async ({ + page, + }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/data.tsx": js` + import { redirect } from '@remix-run/node'; + export function loader({ request }) { + return redirect('/target'); + } + `, + "app/routes/target.tsx": js` + export default function Component() { + return

Target

+ } + `, + }, + }); + + let res = await fixture.requestDocument("/data"); + expect(res.status).toBe(302); + expect(res.headers.get("Location")).toBe("/target"); + expect(await res.text()).toBe(""); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickLink("/data"); + await page.waitForSelector("#target"); + expect(await app.getHtml("#target")).toContain("Target"); + }); + test("processes redirects from handleDataRequest (after loaders)", async ({ page, }) => { @@ -1972,7 +1208,7 @@ test.describe("single-fetch", () => { expect(await app.getHtml("#target")).toContain("Target"); }); - test("processes thrown loader errors via responseStub", async ({ page }) => { + test("processes thrown loader errors", async ({ page }) => { let fixture = await createFixture({ config: { future: { @@ -1984,10 +1220,11 @@ test.describe("single-fetch", () => { "app/routes/data.tsx": js` import { redirect } from '@remix-run/node'; import { isRouteErrorResponse, useRouteError } from '@remix-run/react'; - export function loader({ request, response }) { - response.status = 404; - response.headers.set('X-Foo', 'Bar'); - throw response; + export function headers({ errorHeaders }) { + return errorHeaders; + } + export function loader({ request }) { + throw new Response(null, { status: 404, headers: { 'X-Foo': 'Bar' } }); } export default function Component() { return null @@ -2030,10 +1267,11 @@ test.describe("single-fetch", () => { "app/routes/data.tsx": js` import { redirect } from '@remix-run/node'; import { isRouteErrorResponse, useRouteError } from '@remix-run/react'; - export function action({ request, response }) { - response.status = 404; - response.headers.set('X-Foo', 'Bar'); - throw response; + export function headers({ errorHeaders }) { + return errorHeaders; + } + export function action({ request }) { + throw new Response(null, { status: 404, headers: { 'X-Foo': 'Bar' } }); } export default function Component() { return null @@ -2106,196 +1344,6 @@ test.describe("single-fetch", () => { console.warn = oldConsoleWarn; }); - test("allows resource routes to return null using a response stub", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - export function loader({ response }) { - response.status = 201; - response.headers.set('X-Created-Id', '1'); - return null; - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(201); - expect(res.headers.get("X-Created-Id")).toBe("1"); - expect(await res.text()).toBe(""); - }); - - test("processes response stub onto resource routes returning raw data", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - import { json } from '@remix-run/node'; - - export function loader({ response }) { - // When raw json is returned, the stub status/headers will just be used directly - response.status = 201; - response.headers.set('X-Stub', 'yes') - return { message: "RESOURCE" }; - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(201); - expect(res.headers.get("X-Stub")).toBe("yes"); - expect(await res.json()).toEqual({ - message: "RESOURCE", - }); - }); - - test("processes response stub onto resource routes returning responses", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - import { json } from '@remix-run/node'; - - export function loader({ response }) { - // This will be ignored in favor of the returned Response status - response.status = 200; - response.headers.set('X-Stub', 'yes') - // This will overwrite the returned Response header - response.headers.set('X-Set', '2') - // This will append to the returned Response header - response.headers.append('X-Append', '2') - return json({ message: "RESOURCE" }, { - // This one takes precedence - status: 201, - headers: { - 'X-Response': 'yes', - 'X-Set': '1', - 'X-Append': '1', - }, - }); - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(201); - expect(res.headers.get("X-Response")).toBe("yes"); - expect(res.headers.get("X-Stub")).toBe("yes"); - expect(res.headers.get("X-Set")).toBe("2"); - expect(res.headers.get("X-Append")).toBe("1, 2"); - expect(await res.json()).toEqual({ - message: "RESOURCE", - }); - }); - - test("processes returned response stub redirects from resource route", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - import { json } from '@remix-run/node'; - - export function loader({ response }) { - response.status = 301; - response.headers.set('Location', '/whatever') - return response; - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(301); - expect(res.headers.get("Location")).toBe("/whatever"); - }); - - test("processes thrown response stub redirects from resource route", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - import { json } from '@remix-run/node'; - - export function loader({ response }) { - response.status = 301; - response.headers.set('Location', '/whatever') - throw response; - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(301); - expect(res.headers.get("Location")).toBe("/whatever"); - }); - - test("processes response stub redirects when null is returned from resource route", async () => { - let fixture = await createFixture( - { - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/resource.tsx": js` - import { json } from '@remix-run/node'; - - export function loader({ response }) { - response.status = 301; - response.headers.set('Location', '/whatever') - return null; - } - `, - }, - }, - ServerMode.Development - ); - let res = await fixture.requestResource("/resource"); - expect(res.status).toBe(301); - expect(res.headers.get("Location")).toBe("/whatever"); - }); - test("allows fetcher to hit resource route and return via turbo stream", async ({ page, }) => { @@ -2345,81 +1393,6 @@ test.describe("single-fetch", () => { ); }); - test("does not log thrown redirect response stubs via handleError", async () => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/redirect.tsx": js` - export function action({ response }) { - response.status = 301; - response.headers.set("Location", "/data"); - throw response; - } - export function loader({ response }) { - response.status = 301; - response.headers.set("Location", "/data"); - throw response; - } - export default function Component() { - return

Should not see me

; - } - `, - }, - }); - - let errorLogs = []; - console.error = (e) => errorLogs.push(e); - await fixture.requestDocument("/redirect"); - await fixture.requestSingleFetchData("/redirect.data"); - await fixture.requestSingleFetchData("/redirect.data", { - method: "post", - body: null, - }); - expect(errorLogs.length).toBe(0); - }); - - test("does not log thrown non-redirect response stubs via handleError", async () => { - let fixture = await createFixture({ - config: { - future: { - unstable_singleFetch: true, - }, - }, - files: { - ...files, - "app/routes/redirect.tsx": js` - export function action({ response }) { - response.status = 400; - throw response; - } - export function loader({ response }) { - response.status = 400; - throw response; - } - export default function Component() { - return

Should not see me

; - } - `, - }, - }); - - let errorLogs = []; - console.error = (e) => errorLogs.push(e); - await fixture.requestDocument("/redirect"); - expect(errorLogs.length).toBe(1); // ErrorBoundary render logs this - await fixture.requestSingleFetchData("/redirect.data"); - await fixture.requestSingleFetchData("/redirect.data", { - method: "post", - body: null, - }); - expect(errorLogs.length).toBe(1); - }); - test.describe("client loaders", () => { test("when no routes have client loaders", async ({ page }) => { let fixture = await createFixture( diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index fd81da453b9..517b804e5ad 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -28,7 +28,6 @@ import { createStaticHandlerDataRoutes, createRoutes } from "./routes"; import { createDeferredReadableStream, isRedirectResponse, - isRedirectStatusCode, isResponse, json, } from "./responses"; @@ -36,18 +35,11 @@ import { createServerHandoffString } from "./serverHandoff"; import { getDevServerHooks } from "./dev"; import type { SingleFetchResult, SingleFetchResults } from "./single-fetch"; import { - convertResponseStubToErrorResponse, encodeViaTurboStream, - getResponseStubs, - getSingleFetchDataStrategy, getSingleFetchRedirect, - getSingleFetchResourceRouteDataStrategy, - isResponseStub, - mergeResponseStubs, singleFetchAction, singleFetchLoaders, SingleFetchRedirectSymbol, - ResponseStubOperationsSymbol, SINGLE_FETCH_REDIRECT_STATUS, } from "./single-fetch"; import { resourceRouteJsonWarning } from "./deprecations"; @@ -399,6 +391,7 @@ async function handleSingleFetchRequest( let { result, headers, status } = request.method !== "GET" ? await singleFetchAction( + build, serverMode, staticHandler, request, @@ -407,6 +400,7 @@ async function handleSingleFetchRequest( handleError ) : await singleFetchLoaders( + build, serverMode, staticHandler, request, @@ -447,13 +441,10 @@ async function handleDocumentRequest( criticalCss?: string ) { let context; - let responseStubs = getResponseStubs(); + debugger; try { context = await staticHandler.query(request, { requestContext: loadContext, - unstable_dataStrategy: build.future.unstable_singleFetch - ? getSingleFetchDataStrategy(responseStubs) - : undefined, }); } catch (error: unknown) { handleError(error); @@ -464,37 +455,13 @@ async function handleDocumentRequest( return context; } - let statusCode: number; - let headers: Headers; - if (build.future.unstable_singleFetch) { - let merged = mergeResponseStubs(context, responseStubs); - statusCode = merged.statusCode; - headers = merged.headers; - - if (isRedirectStatusCode(statusCode) && headers.has("Location")) { - return new Response(null, { - status: statusCode, - headers, - }); - } - - if (context.errors) { - for (let [routeId, error] of Object.entries(context.errors)) { - if (isResponseStub(error)) { - context.errors[routeId] = convertResponseStubToErrorResponse(error); - } - } - } - } else { - statusCode = context.statusCode; - headers = getDocumentHeaders(build, context); - } + let headers = getDocumentHeaders(build, context); // Sanitize errors outside of development environments if (context.errors) { Object.values(context.errors).forEach((err) => { // @ts-expect-error `err.error` is "private" from users but intended for internal use - if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) { + if (!isRouteErrorResponse(err) || err.error) { handleError(err); } }); @@ -542,7 +509,7 @@ async function handleDocumentRequest( try { return await handleDocumentRequestFunction( request, - statusCode, + context.statusCode, headers, entryContext, loadContext @@ -636,22 +603,12 @@ async function handleResourceRequest( handleError: (err: unknown) => void ) { try { - let responseStubs = build.future.unstable_singleFetch - ? getResponseStubs() - : {}; // Note we keep the routeId here to align with the Remix handling of // resource routes which doesn't take ?index into account and just takes // the leaf match let response = await staticHandler.queryRoute(request, { routeId, requestContext: loadContext, - ...(build.future.unstable_singleFetch - ? { - unstable_dataStrategy: getSingleFetchResourceRouteDataStrategy({ - responseStubs, - }), - } - : null), }); if (typeof response === "object" && response !== null) { @@ -662,36 +619,14 @@ async function handleResourceRequest( ); } - if (build.future.unstable_singleFetch) { - let stub = responseStubs[routeId]; - if (isResponse(response)) { - // If a response was returned, we use it's status and we merge our - // response stub headers onto it - let ops = stub[ResponseStubOperationsSymbol]; - for (let [op, ...args] of ops) { - // @ts-expect-error - response.headers[op](...args); - } - } else if (isResponseStub(response) || response == null) { - // If the stub or null was returned, then there is no body so we just - // proxy along the status/headers to a Response - response = new Response(null, { - status: stub.status, - headers: stub.headers, - }); - } else { - console.warn( - resourceRouteJsonWarning( - request.method === "GET" ? "loader" : "action", - routeId - ) - ); - // Otherwise we create a json Response using the stub - response = json(response, { - status: stub.status, - headers: stub.headers, - }); - } + if (build.future.unstable_singleFetch && !isResponse(response)) { + console.warn( + resourceRouteJsonWarning( + request.method === "GET" ? "loader" : "action", + routeId + ) + ); + response = json(response); } // callRouteLoader/callRouteAction always return responses (w/o single fetch). @@ -709,13 +644,6 @@ async function handleResourceRequest( return response; } - if (isResponseStub(error)) { - return new Response(null, { - status: error.status, - headers: error.headers, - }); - } - if (isRouteErrorResponse(error)) { if (error) { handleError(error); diff --git a/packages/remix-server-runtime/single-fetch.ts b/packages/remix-server-runtime/single-fetch.ts index 4c31135c573..9582804ea8f 100644 --- a/packages/remix-server-runtime/single-fetch.ts +++ b/packages/remix-server-runtime/single-fetch.ts @@ -1,10 +1,7 @@ import type { - ActionFunctionArgs as RRActionArgs, - LoaderFunctionArgs as RRLoaderArgs, StaticHandler, unstable_DataStrategyFunctionArgs as DataStrategyFunctionArgs, unstable_DataStrategyFunction as DataStrategyFunction, - StaticHandlerContext, } from "@remix-run/router"; import { isRouteErrorResponse, @@ -12,15 +9,17 @@ import { } from "@remix-run/router"; import { encode } from "turbo-stream"; +import type { ServerBuild } from "./build"; import type { AppLoadContext } from "./data"; import { sanitizeError, sanitizeErrors } from "./errors"; +import { getDocumentHeaders } from "./headers"; import { ServerMode } from "./mode"; import type { TypedDeferredData, TypedResponse } from "./responses"; -import { isDeferredData, isRedirectStatusCode, isResponse } from "./responses"; +import { isRedirectStatusCode, isResponse } from "./responses"; +import type { ActionFunctionArgs, LoaderFunctionArgs } from "./routeModules"; import type { SerializeFrom } from "./serialize"; export const SingleFetchRedirectSymbol = Symbol("SingleFetchRedirect"); -const ResponseStubActionSymbol = Symbol("ResponseStubAction"); type SingleFetchRedirectResult = { redirect: string; @@ -38,10 +37,6 @@ export type SingleFetchResults = { [SingleFetchRedirectSymbol]?: SingleFetchRedirectResult; }; -export type DataStrategyCtx = { - response: ResponseStub; -}; - // We can't use a 3xx status or else the `fetch()` would follow the redirect. // We need to communicate the redirect back as data so we can act on it in the // client side router. We use a 202 to avoid any automatic caching we might @@ -49,13 +44,13 @@ export type DataStrategyCtx = { // the user control cache behavior via Cache-Control export const SINGLE_FETCH_REDIRECT_STATUS = 202; -export function getSingleFetchDataStrategy( - responseStubs: ReturnType, - { - isActionDataRequest, - loadRouteIds, - }: { isActionDataRequest?: boolean; loadRouteIds?: string[] } = {} -): DataStrategyFunction { +export function getSingleFetchDataStrategy({ + isActionDataRequest, + loadRouteIds, +}: { + isActionDataRequest?: boolean; + loadRouteIds?: string[]; +} = {}): DataStrategyFunction { return async ({ request, matches }: DataStrategyFunctionArgs) => { // Don't call loaders on action data requests if (isActionDataRequest && request.method === "GET") { @@ -68,63 +63,12 @@ export function getSingleFetchDataStrategy( let results = await Promise.all( matches.map(async (match) => { - let responseStub: ResponseStubImpl | undefined; - if (request.method !== "GET") { - responseStub = responseStubs[ResponseStubActionSymbol]; - } else { - responseStub = responseStubs[match.route.id]; - } - let result = await match.resolve(async (handler) => { - // Cast `ResponseStubImpl -> ResponseStub` to hide the symbol in userland - let ctx: DataStrategyCtx = { response: responseStub as ResponseStub }; // Only run opt-in loaders when fine-grained revalidation is enabled let data = loadRouteIds && !loadRouteIds.includes(match.route.id) ? null - : await handler(ctx); - return { type: "data", result: data }; - }); - - // Transfer raw Response status/headers to responseStubs - if (isResponse(result.result)) { - proxyResponseToResponseStub( - result.result.status, - result.result.headers, - responseStub - ); - } else if (isDeferredData(result.result) && result.result.init) { - proxyResponseToResponseStub( - result.result.init.status, - new Headers(result.result.init.headers), - responseStub - ); - } - - return result; - }) - ); - return results; - }; -} - -export function getSingleFetchResourceRouteDataStrategy({ - responseStubs, -}: { - responseStubs: ReturnType; -}): DataStrategyFunction { - return async ({ matches }: DataStrategyFunctionArgs) => { - let results = await Promise.all( - matches.map(async (match) => { - let responseStub = match.shouldLoad - ? responseStubs[match.route.id] - : null; - let result = await match.resolve(async (handler) => { - // Cast `ResponseStubImpl -> ResponseStub` to hide the symbol in userland - let ctx: DataStrategyCtx = { - response: responseStub as ResponseStub, - }; - let data = await handler(ctx); + : await handler(); return { type: "data", result: data }; }); return result; @@ -135,6 +79,7 @@ export function getSingleFetchResourceRouteDataStrategy({ } export async function singleFetchAction( + build: ServerBuild, serverMode: ServerMode, staticHandler: StaticHandler, request: Request, @@ -151,11 +96,10 @@ export async function singleFetchAction( ...(request.body ? { duplex: "half" } : undefined), }); - let responseStubs = getResponseStubs(); let result = await staticHandler.query(handlerRequest, { requestContext: loadContext, skipLoaderErrorBubbling: true, - unstable_dataStrategy: getSingleFetchDataStrategy(responseStubs, { + unstable_dataStrategy: getSingleFetchDataStrategy({ isActionDataRequest: true, }), }); @@ -171,15 +115,11 @@ export async function singleFetchAction( } let context = result; + let headers = getDocumentHeaders(build, context); - let singleFetchResult: SingleFetchResult; - let { statusCode, headers } = mergeResponseStubs(context, responseStubs, { - isActionDataRequest: true, - }); - - if (isRedirectStatusCode(statusCode) && headers.has("Location")) { + if (isRedirectStatusCode(context.statusCode) && headers.has("Location")) { return { - result: getSingleFetchRedirect(statusCode, headers), + result: getSingleFetchRedirect(context.statusCode, headers), headers, status: SINGLE_FETCH_REDIRECT_STATUS, }; @@ -189,20 +129,16 @@ export async function singleFetchAction( if (context.errors) { Object.values(context.errors).forEach((err) => { // @ts-expect-error This is "private" from users but intended for internal use - if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) { + if (!isRouteErrorResponse(err) || err.error) { handleError(err); } }); context.errors = sanitizeErrors(context.errors, serverMode); } + let singleFetchResult: SingleFetchResult; if (context.errors) { - let error = Object.values(context.errors)[0]; - singleFetchResult = { - error: isResponseStub(error) - ? convertResponseStubToErrorResponse(error) - : error, - }; + singleFetchResult = { error: Object.values(context.errors)[0] }; } else { singleFetchResult = { data: Object.values(context.actionData || {})[0] }; } @@ -210,7 +146,7 @@ export async function singleFetchAction( return { result: singleFetchResult, headers, - status: statusCode, + status: context.statusCode, }; } catch (error) { handleError(error); @@ -224,6 +160,7 @@ export async function singleFetchAction( } export async function singleFetchLoaders( + build: ServerBuild, serverMode: ServerMode, staticHandler: StaticHandler, request: Request, @@ -232,6 +169,7 @@ export async function singleFetchLoaders( handleError: (err: unknown) => void ): Promise<{ result: SingleFetchResults; headers: Headers; status: number }> { try { + debugger; let handlerRequest = new Request(handlerUrl, { headers: request.headers, signal: request.signal, @@ -239,11 +177,10 @@ export async function singleFetchLoaders( let loadRouteIds = new URL(request.url).searchParams.get("_routes")?.split(",") || undefined; - let responseStubs = getResponseStubs(); let result = await staticHandler.query(handlerRequest, { requestContext: loadContext, skipLoaderErrorBubbling: true, - unstable_dataStrategy: getSingleFetchDataStrategy(responseStubs, { + unstable_dataStrategy: getSingleFetchDataStrategy({ loadRouteIds, }), }); @@ -262,14 +199,13 @@ export async function singleFetchLoaders( } let context = result; + let headers = getDocumentHeaders(build, context); - let { statusCode, headers } = mergeResponseStubs(context, responseStubs); - - if (isRedirectStatusCode(statusCode) && headers.has("Location")) { + if (isRedirectStatusCode(context.statusCode) && headers.has("Location")) { return { result: { [SingleFetchRedirectSymbol]: getSingleFetchRedirect( - statusCode, + context.statusCode, headers ), }, @@ -282,7 +218,7 @@ export async function singleFetchLoaders( if (context.errors) { Object.values(context.errors).forEach((err) => { // @ts-expect-error This is "private" from users but intended for internal use - if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) { + if (!isRouteErrorResponse(err) || err.error) { handleError(err); } }); @@ -302,13 +238,7 @@ export async function singleFetchLoaders( let data = context.loaderData?.[m.route.id]; let error = context.errors?.[m.route.id]; if (error !== undefined) { - if (isResponseStub(error)) { - results[m.route.id] = { - error: convertResponseStubToErrorResponse(error), - }; - } else { - results[m.route.id] = { error }; - } + results[m.route.id] = { error }; } else if (data !== undefined) { results[m.route.id] = { data }; } @@ -317,7 +247,7 @@ export async function singleFetchLoaders( return { result: results, headers, - status: statusCode, + status: context.statusCode, }; } catch (error: unknown) { handleError(error); @@ -330,123 +260,6 @@ export async function singleFetchLoaders( } } -export function isResponseStub(value: any): value is ResponseStubImpl { - return ( - value && typeof value === "object" && ResponseStubOperationsSymbol in value - ); -} - -function getResponseStub(status?: number) { - let headers = new Headers(); - let operations: ResponseStubOperation[] = []; - let headersProxy = new Proxy(headers, { - get(target, prop, receiver) { - if (prop === "set" || prop === "append" || prop === "delete") { - return (name: string, value: string) => { - operations.push([prop, name, value]); - Reflect.apply(target[prop], target, [name, value]); - }; - } - return Reflect.get(target, prop, receiver); - }, - }); - return { - status, - headers: headersProxy, - [ResponseStubOperationsSymbol]: operations, - }; -} - -export function getResponseStubs() { - return new Proxy({} as Record, { - get(responseStubCache, prop) { - let cached = responseStubCache[prop]; - if (!cached) { - responseStubCache[prop] = cached = getResponseStub(); - } - return cached; - }, - }); -} - -function proxyResponseToResponseStub( - status: number | undefined, - headers: Headers, - responseStub: ResponseStubImpl -) { - if (status != null && responseStub.status == null) { - responseStub.status = status; - } - for (let [k, v] of headers) { - if (k.toLowerCase() !== "set-cookie") { - responseStub.headers.set(k, v); - } - } - - // Unsure why this is complaining? It's fine in VSCode but fails with tsc... - // @ts-ignore - ignoring instead of expecting because otherwise build fails locally - for (let v of headers.getSetCookie()) { - responseStub.headers.append("Set-Cookie", v); - } -} - -export function convertResponseStubToErrorResponse(stub: ResponseStub) { - return new ErrorResponseImpl(stub.status || 500, "", null); -} - -export function mergeResponseStubs( - context: StaticHandlerContext, - responseStubs: ReturnType, - { isActionDataRequest }: { isActionDataRequest?: boolean } = {} -) { - let statusCode: number | undefined = undefined; - let headers = new Headers(); - - // Action followed by top-down loaders - let actionStub = responseStubs[ResponseStubActionSymbol]; - let stubs = [actionStub]; - - // Nothing to merge at the route level on action data requests - if (!isActionDataRequest) { - stubs.push(...context.matches.map((m) => responseStubs[m.route.id])); - } - - for (let stub of stubs) { - // Take the highest error/redirect, or the lowest success value - preferring - // action 200's over loader 200s - if ( - // first status found on the way down - (statusCode === undefined && stub.status) || - // deeper 2xx status found while not overriding the action status - (statusCode !== undefined && - statusCode < 300 && - stub.status && - statusCode !== actionStub?.status) - ) { - statusCode = stub.status; - } - - // Replay headers operations in order - let ops = stub[ResponseStubOperationsSymbol]; - for (let [op, ...args] of ops) { - // @ts-expect-error - headers[op](...args); - } - } - - // If no response stubs set it, use whatever we got back from the router - // context which handles internal ErrorResponse cases like 404/405's where - // we may never run a loader/action - if (statusCode === undefined) { - statusCode = context.statusCode; - } - if (statusCode === undefined) { - statusCode = 200; - } - - return { statusCode, headers }; -} - export function getSingleFetchRedirect( status: number, headers: Headers @@ -555,44 +368,12 @@ export type Serialize = Awaited> extends TypedResponse> ? SerializeFrom : Awaited>; -export const ResponseStubOperationsSymbol = Symbol("ResponseStubOperations"); -export type ResponseStubOperation = [ - "set" | "append" | "delete", - string, - string? -]; - -/** - * A stubbed response to let you set the status/headers of your response from - * loader/action functions - */ -export type ResponseStub = { - status: number | undefined; - headers: Headers; -}; - -export type ResponseStubImpl = ResponseStub & { - [ResponseStubOperationsSymbol]: ResponseStubOperation[]; -}; - -// loader -type LoaderArgs = RRLoaderArgs & { - // Context is always provided in Remix, and typed for module augmentation support. - context: AppLoadContext; - response: ResponseStub; -}; export type Loader = ( - args: LoaderArgs + args: LoaderFunctionArgs ) => MaybePromise; export let defineLoader = (loader: T): T => loader; -// action -type ActionArgs = RRActionArgs & { - // Context is always provided in Remix, and typed for module augmentation support. - context: AppLoadContext; - response: ResponseStub; -}; export type Action = ( - args: ActionArgs + args: ActionFunctionArgs ) => MaybePromise; export let defineAction = (action: T): T => action; From 0edb0ed08efe5cfc91a5dd863af0e7327ca49f74 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jul 2024 18:11:05 -0400 Subject: [PATCH 02/13] Remove a few leftover dead code paths --- packages/remix-server-runtime/data.ts | 11 ----------- packages/remix-server-runtime/routeModules.ts | 5 ----- packages/remix-server-runtime/routes.ts | 5 ----- 3 files changed, 21 deletions(-) diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts index bdca6350064..e4c0ffc4e6b 100644 --- a/packages/remix-server-runtime/data.ts +++ b/packages/remix-server-runtime/data.ts @@ -11,7 +11,6 @@ import type { LoaderFunction, LoaderFunctionArgs, } from "./routeModules"; -import type { ResponseStub } from "./single-fetch"; /** * An object of unknown type for route loaders and actions provided by the @@ -35,7 +34,6 @@ export async function callRouteAction({ request, routeId, singleFetch, - response, }: { request: Request; action: ActionFunction; @@ -43,15 +41,11 @@ export async function callRouteAction({ loadContext: AppLoadContext; routeId: string; singleFetch: boolean; - response?: ResponseStub; }) { let result = await action({ request: stripDataParam(stripIndexParam(request)), context: loadContext, params, - // Only provided when single fetch is enabled, and made available via - // `defineAction` types, not `ActionFunctionArgs` - ...(singleFetch ? { response } : null), }); if (result === undefined) { @@ -76,7 +70,6 @@ export async function callRouteLoader({ request, routeId, singleFetch, - response, }: { request: Request; loader: LoaderFunction; @@ -84,15 +77,11 @@ export async function callRouteLoader({ loadContext: AppLoadContext; routeId: string; singleFetch: boolean; - response?: ResponseStub; }) { let result = await loader({ request: stripDataParam(stripIndexParam(request)), context: loadContext, params, - // Only provided when single fetch is enabled, and made available via - // `defineLoader` types, not `LoaderFunctionArgs` - ...(singleFetch ? { response } : null), }); if (result === undefined) { diff --git a/packages/remix-server-runtime/routeModules.ts b/packages/remix-server-runtime/routeModules.ts index 8bfb68173f4..58e0ce2f7d0 100644 --- a/packages/remix-server-runtime/routeModules.ts +++ b/packages/remix-server-runtime/routeModules.ts @@ -11,7 +11,6 @@ import type { import type { AppData, AppLoadContext } from "./data"; import type { LinkDescriptor } from "./links"; import type { SerializeFrom } from "./serialize"; -import type { ResponseStub } from "./single-fetch"; export interface RouteModules { [routeId: string]: RouteModule | undefined; @@ -41,8 +40,6 @@ export type ActionFunction = ( export type ActionFunctionArgs = RRActionFunctionArgs & { // Context is always provided in Remix, and typed for module augmentation support. context: AppLoadContext; - // TODO: (v7) Make this non-optional once single-fetch is the default - response?: ResponseStub; }; /** @@ -74,8 +71,6 @@ export type LoaderFunction = ( export type LoaderFunctionArgs = RRLoaderFunctionArgs & { // Context is always provided in Remix, and typed for module augmentation support. context: AppLoadContext; - // TODO: (v7) Make this non-optional once single-fetch is the default - response?: ResponseStub; }; /** diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts index 89736b4533a..029b98469d0 100644 --- a/packages/remix-server-runtime/routes.ts +++ b/packages/remix-server-runtime/routes.ts @@ -7,7 +7,6 @@ import type { import { callRouteAction, callRouteLoader } from "./data"; import type { FutureConfig } from "./entry"; import type { ServerRouteModule } from "./routeModules"; -import type { DataStrategyCtx } from "./single-fetch"; export interface RouteManifest { [routeId: string]: Route; @@ -101,8 +100,6 @@ export function createStaticHandlerDataRoutes( loader: route.module.loader!, routeId: route.id, singleFetch: future.unstable_singleFetch === true, - response: (dataStrategyCtx as DataStrategyCtx | undefined) - ?.response, }) : undefined, action: route.module.action @@ -114,8 +111,6 @@ export function createStaticHandlerDataRoutes( action: route.module.action!, routeId: route.id, singleFetch: future.unstable_singleFetch === true, - response: (dataStrategyCtx as DataStrategyCtx | undefined) - ?.response, }) : undefined, handle: route.module.handle, From a5bb161d3af997abc3aaae7ff5b489b92007a624 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 18 Jul 2024 09:43:22 -0400 Subject: [PATCH 03/13] Try to fix wrangler error? --- integration/helpers/vite.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/integration/helpers/vite.ts b/integration/helpers/vite.ts index 65c805288ff..32beed34620 100644 --- a/integration/helpers/vite.ts +++ b/integration/helpers/vite.ts @@ -185,7 +185,16 @@ export const wranglerPagesDev = async ({ let proc = spawn( nodeBin, - [wranglerBin, "pages", "dev", "./build/client", "--port", String(port)], + [ + wranglerBin, + "pages", + "dev", + "./build/client", + "--port", + String(port), + "--compatibility-date", + "2024-01-29", + ], { cwd, stdio: "pipe", From 7d7aa65eec0fd9a8ca0575580f1a99e99e8371e3 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 13:10:35 -0400 Subject: [PATCH 04/13] Bump router --- integration/package.json | 2 +- packages/remix-dev/package.json | 2 +- packages/remix-react/package.json | 6 +-- packages/remix-server-runtime/package.json | 2 +- packages/remix-testing/package.json | 4 +- pnpm-lock.yaml | 50 +++++++++++----------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/integration/package.json b/integration/package.json index db21212335a..baa3e0a8054 100644 --- a/integration/package.json +++ b/integration/package.json @@ -14,7 +14,7 @@ "@remix-run/dev": "workspace:*", "@remix-run/express": "workspace:*", "@remix-run/node": "workspace:*", - "@remix-run/router": "0.0.0-experimental-cffa549a1", + "@remix-run/router": "0.0.0-experimental-5df42afed", "@remix-run/server-runtime": "workspace:*", "@types/express": "^4.17.9", "@vanilla-extract/css": "^1.10.0", diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 3aa01012e73..94ea24dcb1c 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -32,7 +32,7 @@ "@mdx-js/mdx": "^2.3.0", "@npmcli/package-json": "^4.0.1", "@remix-run/node": "workspace:*", - "@remix-run/router": "0.0.0-experimental-cffa549a1", + "@remix-run/router": "0.0.0-experimental-5df42afed", "@remix-run/server-runtime": "workspace:*", "@types/mdx": "^2.0.5", "@vanilla-extract/integration": "^6.2.0", diff --git a/packages/remix-react/package.json b/packages/remix-react/package.json index 1baa7a6c2f1..ea025f0035a 100644 --- a/packages/remix-react/package.json +++ b/packages/remix-react/package.json @@ -19,10 +19,10 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "0.0.0-experimental-cffa549a1", + "@remix-run/router": "0.0.0-experimental-5df42afed", "@remix-run/server-runtime": "workspace:*", - "react-router": "0.0.0-experimental-cffa549a1", - "react-router-dom": "0.0.0-experimental-cffa549a1", + "react-router": "0.0.0-experimental-5df42afed", + "react-router-dom": "0.0.0-experimental-5df42afed", "turbo-stream": "2.2.0" }, "devDependencies": { diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json index 148fd0f0460..6ce5fb0d9a3 100644 --- a/packages/remix-server-runtime/package.json +++ b/packages/remix-server-runtime/package.json @@ -19,7 +19,7 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "0.0.0-experimental-cffa549a1", + "@remix-run/router": "0.0.0-experimental-5df42afed", "@types/cookie": "^0.6.0", "@web3-storage/multipart-parser": "^1.0.0", "cookie": "^0.6.0", diff --git a/packages/remix-testing/package.json b/packages/remix-testing/package.json index cfab24a48fd..b2baa0959e1 100644 --- a/packages/remix-testing/package.json +++ b/packages/remix-testing/package.json @@ -21,8 +21,8 @@ "dependencies": { "@remix-run/node": "workspace:*", "@remix-run/react": "workspace:*", - "@remix-run/router": "0.0.0-experimental-cffa549a1", - "react-router-dom": "0.0.0-experimental-cffa549a1" + "@remix-run/router": "0.0.0-experimental-5df42afed", + "react-router-dom": "0.0.0-experimental-5df42afed" }, "devDependencies": { "@remix-run/server-runtime": "workspace:*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 78decb38b4c..d5d4488bdae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -323,8 +323,8 @@ importers: specifier: workspace:* version: link:../packages/remix-node '@remix-run/router': - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1 + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed '@remix-run/server-runtime': specifier: workspace:* version: link:../packages/remix-server-runtime @@ -871,8 +871,8 @@ importers: specifier: ^2.10.3 version: link:../remix-react '@remix-run/router': - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1 + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime @@ -1217,17 +1217,17 @@ importers: packages/remix-react: dependencies: '@remix-run/router': - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1 + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime react-router: - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1(react@18.2.0) + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed(react@18.2.0) react-router-dom: - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1(react-dom@18.2.0)(react@18.2.0) + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0) turbo-stream: specifier: 2.2.0 version: 2.2.0 @@ -1303,8 +1303,8 @@ importers: packages/remix-server-runtime: dependencies: '@remix-run/router': - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1 + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed '@types/cookie': specifier: ^0.6.0 version: 0.6.0 @@ -1340,11 +1340,11 @@ importers: specifier: workspace:* version: link:../remix-react '@remix-run/router': - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1 + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed react-router-dom: - specifier: 0.0.0-experimental-cffa549a1 - version: 0.0.0-experimental-cffa549a1(react-dom@18.2.0)(react@18.2.0) + specifier: 0.0.0-experimental-5df42afed + version: 0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0) devDependencies: '@remix-run/server-runtime': specifier: workspace:* @@ -4201,8 +4201,8 @@ packages: - encoding dev: false - /@remix-run/router@0.0.0-experimental-cffa549a1: - resolution: {integrity: sha512-Pn7hkGb4NL91+wMKidAvVUxLjjWeidhBe66rfQG04BDQHoCsBvncM54KtymGprCdjM1ki06c9kcNeR3fz9rDsA==} + /@remix-run/router@0.0.0-experimental-5df42afed: + resolution: {integrity: sha512-J2cSDEmAxP2gxTi7U4zePG5frP/BKpB6AtUpITTZomFoOoCfz81HzBTJak4NcCsS1jCFgukRIZbBW8IvkESD/Q==} engines: {node: '>=14.0.0'} dev: false @@ -12786,26 +12786,26 @@ packages: engines: {node: '>=0.10.0'} dev: false - /react-router-dom@0.0.0-experimental-cffa549a1(react-dom@18.2.0)(react@18.2.0): - resolution: {integrity: sha512-qnObsw+nV5pgoObJ6e+PHG8pltAvpeuqtHQX/Z8VtjQTPcXhLhXysvaE2JlQGxUNnE7OnJCLLbtk2722UvK1bQ==} + /react-router-dom@0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0): + resolution: {integrity: sha512-1B1Qb5Qw+b326WoRfsDXPs3anb45aFvRb8btbpdNBodp1ZkPwS0h1Y5TNcOoo81FvBPeSOydmFmziCpFR58hYg==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' react-dom: '>=16.8' dependencies: - '@remix-run/router': 0.0.0-experimental-cffa549a1 + '@remix-run/router': 0.0.0-experimental-5df42afed react: 18.2.0 react-dom: 18.2.0(react@18.2.0) - react-router: 0.0.0-experimental-cffa549a1(react@18.2.0) + react-router: 0.0.0-experimental-5df42afed(react@18.2.0) dev: false - /react-router@0.0.0-experimental-cffa549a1(react@18.2.0): - resolution: {integrity: sha512-KAdzysntJa81nnnXkm06YowOjt62hNbLph+IH7CLltLFKKdq420fdSUxZ79olJpgWEKG9fjeqLr4X/pJCEyUrg==} + /react-router@0.0.0-experimental-5df42afed(react@18.2.0): + resolution: {integrity: sha512-GLkDE+AZdgWFwTg4zpAEwZsSHK0ZNvZdkjSqcPkH9fYBJqvu7aOK+hatx6x+7J7HiTij0U1sr8CsUDYMTsTEUw==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' dependencies: - '@remix-run/router': 0.0.0-experimental-cffa549a1 + '@remix-run/router': 0.0.0-experimental-5df42afed react: 18.2.0 dev: false From 55fa5dd0dc75a7c7d69e1da4a5ceb33c97bcb545 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 13:11:10 -0400 Subject: [PATCH 05/13] Add support for unstabl_data --- integration/single-fetch-test.ts | 194 ++++++++++++++++++ packages/remix-cloudflare/index.ts | 1 + packages/remix-deno/index.ts | 1 + packages/remix-node/index.ts | 1 + packages/remix-react/index.tsx | 1 + packages/remix-react/single-fetch.tsx | 21 +- packages/remix-server-runtime/index.ts | 1 + packages/remix-server-runtime/server.ts | 1 - packages/remix-server-runtime/single-fetch.ts | 6 +- 9 files changed, 214 insertions(+), 13 deletions(-) diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index c3cda418b20..e383ff812ab 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -94,6 +94,48 @@ const files = { ) } `, + + "app/routes/data-with-response.tsx": js` + import { useActionData, useLoaderData, unstable_data as data } from "@remix-run/react"; + + export function headers ({ actionHeaders, loaderHeaders, errorHeaders }) { + if ([...actionHeaders].length > 0) { + return actionHeaders; + } else { + return loaderHeaders; + } + } + + export async function action({ request }) { + let formData = await request.formData(); + return data({ + key: formData.get('key'), + }, { status: 201, headers: { 'X-Action': 'yes' }}); + } + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("error")) { + throw new Error("Loader Error"); + } + return data({ + message: "DATA", + date: new Date("${ISO_DATE}"), + }, { status: 206, headers: { 'X-Loader': 'yes' }}); + } + + export default function DataWithResponse() { + let data = useLoaderData(); + let actionData = useActionData(); + return ( + <> +

Data

+

{data.message}

+

{data.date.toISOString()}

+ {actionData ?

{actionData.key}

: null} + + ) + } + `, }; test.describe("single-fetch", () => { @@ -200,6 +242,57 @@ test.describe("single-fetch", () => { }); }); + test("loads proper data (via unstable_data) on single fetch loader requests", async () => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files, + }); + let res = await fixture.requestSingleFetchData("/data-with-response.data"); + expect(res.status).toEqual(206); + expect(res.headers.get("X-Loader")).toEqual("yes"); + expect(res.data).toEqual({ + root: { + data: { + message: "ROOT", + }, + }, + "routes/data-with-response": { + data: { + message: "DATA", + date: new Date(ISO_DATE), + }, + }, + }); + }); + + test("loads proper data (via unstable_data) on single fetch action requests", async () => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files, + }); + let postBody = new URLSearchParams(); + postBody.set("key", "value"); + let res = await fixture.requestSingleFetchData("/data-with-response.data", { + method: "post", + body: postBody, + }); + expect(res.status).toEqual(201); + expect(res.headers.get("X-Action")).toEqual("yes"); + expect(res.data).toEqual({ + data: { + key: "value", + }, + }); + }); + test("loads proper data on document request", async ({ page }) => { let fixture = await createFixture({ config: { @@ -431,6 +524,107 @@ test.describe("single-fetch", () => { expect(urls).toEqual([]); }); + test("does not revalidate on 4xx/5xx action responses (via unstable_data)", async ({ + page, + }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/action.tsx": js` + import { Form, Link, useActionData, useLoaderData, useNavigation, unstable_data } from '@remix-run/react'; + + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "5xx") { + throw unstable_data("Thrown 500", { status: 500 }); + } + if (fd.get('throw') === "4xx") { + throw unstable_data("Thrown 400", { status: 400 }); + } + if (fd.get('return') === "5xx") { + return unstable_data("Returned 500", { status: 500 }); + } + if (fd.get('return') === "4xx") { + return unstable_data("Returned 400", { status: 400 }); + } + return null; + } + + let count = 0; + export function loader() { + return { count: ++count }; + } + + export default function Comp() { + let navigation = useNavigation(); + let data = useLoaderData(); + return ( +
+ + + + +

{data.count}

+ {navigation.state === "idle" ?

idle

: null} +
+ ); + } + + export function ErrorBoundary() { + return ( +
+

Error

+ Back +
+ ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + console.error = () => {}; + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/action"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="return"][value="5xx"]'); + await page.waitForSelector("#idle"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="return"][value="4xx"]'); + await page.waitForSelector("#idle"); + expect(await app.getHtml("#data")).toContain("1"); + expect(urls).toEqual([]); + + await page.click('button[name="throw"][value="5xx"]'); + await page.waitForSelector("#error"); + expect(urls).toEqual([]); + + await app.clickLink("/action"); + await page.waitForSelector("#data"); + expect(await app.getHtml("#data")).toContain("2"); + urls = []; + + await page.click('button[name="throw"][value="4xx"]'); + await page.waitForSelector("#error"); + expect(urls).toEqual([]); + }); test("returns headers correctly for singular loader and action calls", async () => { let fixture = await createFixture({ config: { diff --git a/packages/remix-cloudflare/index.ts b/packages/remix-cloudflare/index.ts index 22b878fd7f3..996f5f6749e 100644 --- a/packages/remix-cloudflare/index.ts +++ b/packages/remix-cloudflare/index.ts @@ -12,6 +12,7 @@ export { export { createRequestHandler, createSession, + unstable_data, unstable_defineLoader, unstable_defineAction, defer, diff --git a/packages/remix-deno/index.ts b/packages/remix-deno/index.ts index 5652d74ac06..7e7cbbb2c09 100644 --- a/packages/remix-deno/index.ts +++ b/packages/remix-deno/index.ts @@ -27,6 +27,7 @@ export { replace, unstable_composeUploadHandlers, unstable_createMemoryUploadHandler, + unstable_data, unstable_defineAction, unstable_defineLoader, unstable_parseMultipartFormData, diff --git a/packages/remix-node/index.ts b/packages/remix-node/index.ts index 262b1a67446..0665a313185 100644 --- a/packages/remix-node/index.ts +++ b/packages/remix-node/index.ts @@ -24,6 +24,7 @@ export { export { createRequestHandler, createSession, + unstable_data, unstable_defineLoader, unstable_defineAction, defer, diff --git a/packages/remix-react/index.tsx b/packages/remix-react/index.tsx index 5d1dda5bcb8..8d8809ca644 100644 --- a/packages/remix-react/index.tsx +++ b/packages/remix-react/index.tsx @@ -66,6 +66,7 @@ export { redirect, redirectDocument, replace, + unstable_data, } from "@remix-run/server-runtime"; export type { RemixBrowserProps } from "./browser"; diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx index e6c156efa95..0c512fc1439 100644 --- a/packages/remix-react/single-fetch.tsx +++ b/packages/remix-react/single-fetch.tsx @@ -8,6 +8,7 @@ import type { import { UNSAFE_ErrorResponseImpl as ErrorResponseImpl, redirect, + unstable_data, } from "@remix-run/router"; import type { UNSAFE_SingleFetchResult as SingleFetchResult, @@ -23,7 +24,7 @@ import type { } from "react-router-dom"; import { decode } from "turbo-stream"; -import { createRequestInit } from "./data"; +import { createRequestInit, isResponse } from "./data"; import type { AssetsManifest, EntryContext } from "./entry"; import { escapeHtml } from "./markup"; import type { RouteModules } from "./routeModules"; @@ -163,17 +164,15 @@ function singleFetchActionStrategy( actionStatus = status; return unwrapSingleFetchResult(data as SingleFetchResult, m.route.id); }); - return { - type: "data", - result, - status: actionStatus, - }; + // If we have a response here, like a redirect, return it as is + if (isResponse(result)) { + return { type: "data", result }; + } + // Otherwise, proxy along the statusCode via unstable_data() + // (most notably for skipping action error revalidation) + return { type: "data", result: unstable_data(result, actionStatus) }; }); - return { - ...result, - // Proxy along the action HTTP response status for thrown errors - status: actionStatus, - }; + return result; }) ); } diff --git a/packages/remix-server-runtime/index.ts b/packages/remix-server-runtime/index.ts index c1fbfd564b2..b4ec1d6678b 100644 --- a/packages/remix-server-runtime/index.ts +++ b/packages/remix-server-runtime/index.ts @@ -8,6 +8,7 @@ export { defer, json, redirect, redirectDocument, replace } from "./responses"; export { SingleFetchRedirectSymbol as UNSAFE_SingleFetchRedirectSymbol, + data as unstable_data, defineLoader as unstable_defineLoader, defineAction as unstable_defineAction, } from "./single-fetch"; diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 517b804e5ad..35c2ebfef58 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -441,7 +441,6 @@ async function handleDocumentRequest( criticalCss?: string ) { let context; - debugger; try { context = await staticHandler.query(request, { requestContext: loadContext, diff --git a/packages/remix-server-runtime/single-fetch.ts b/packages/remix-server-runtime/single-fetch.ts index a1a7fe5e3d1..f6b8957a011 100644 --- a/packages/remix-server-runtime/single-fetch.ts +++ b/packages/remix-server-runtime/single-fetch.ts @@ -5,6 +5,7 @@ import type { } from "@remix-run/router"; import { isRouteErrorResponse, + unstable_data as routerData, UNSAFE_ErrorResponseImpl as ErrorResponseImpl, } from "@remix-run/router"; import { encode } from "turbo-stream"; @@ -170,7 +171,6 @@ export async function singleFetchLoaders( handleError: (err: unknown) => void ): Promise<{ result: SingleFetchResults; headers: Headers; status: number }> { try { - debugger; let handlerRequest = new Request(handlerUrl, { headers: request.headers, signal: request.signal, @@ -335,6 +335,10 @@ export function encodeViaTurboStream( }); } +export function data(value: Serializable, init?: number | ResponseInit) { + return routerData(value, init); +} + type MaybePromise = T | Promise; type Serializable = From ddc078b2f6fec7d41982f8b51181e5eda53b8c50 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 13:19:29 -0400 Subject: [PATCH 06/13] Fix unit test --- packages/remix-react/__tests__/exports-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/remix-react/__tests__/exports-test.tsx b/packages/remix-react/__tests__/exports-test.tsx index 3cdb38d2e9c..953b9cd0f6b 100644 --- a/packages/remix-react/__tests__/exports-test.tsx +++ b/packages/remix-react/__tests__/exports-test.tsx @@ -40,6 +40,7 @@ let modifiedExports = new Set([ "json", // types "redirect", // types "redirectDocument", // types + "replace", // types "useActionData", // types "useFetcher", // types "useLoaderData", // types From 02c80323bb3a80d850cb6451db9c6526706d3463 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 13:32:27 -0400 Subject: [PATCH 07/13] Add more deets to changeset --- .changeset/polite-pumas-visit.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.changeset/polite-pumas-visit.md b/.changeset/polite-pumas-visit.md index 1774910e3fb..f1207c6de0d 100644 --- a/.changeset/polite-pumas-visit.md +++ b/.changeset/polite-pumas-visit.md @@ -3,3 +3,21 @@ --- Single Fetch: Remove `responseStub` in favor of `headers` + +* Background + * The original Single Fetch approach was based on an assumption that an eventual `middleware` implementation would require something like `ResponseStub` so users could mutate `status`/`headers` in `middleware` before/after handlers as well as during handlers + * We wanted to align how `headers` got merged between document and data requests + * So we made document requests also use `ResponseStub` and removed the usage of `headers` in Single Fetch + * The realization/alignment between Michael and Ryan on the recent [roadmap planning](https://www.youtube.com/watch?v=f5z_axCofW0) made us realize that the original assumption was incorrect + * `middleware` won't need a stub - users can just mutate the `Response` they get from `await next()` directly + * With that gone, and still wanting to align how `headers` get merged, it makes more sense to stick with the current `headers` API and apply that to Single Fetch and avoid introducing a totally new thing in `RepsonseStub` (that always felt a bit awkward to work with anyway) + +* With this change: + * You are encouraged to stop returning `Response` instances in favor of returning raw data from loaders and actions: + * ~~`return json({ data: whatever });`~~ + * `return { data: whatever };` + * In most cases, you can remove your `json()` and `defer()` calls in favor of returning raw data if they weren't setting custom `status`/`headers` + * We will be removing both `json` and `defer` in the next major version, but both _should_ still work in Single Fetch in v2 to allow for incremental adoption of the new behavior + * If you need custom `status`/`headers`: + * We've added a new `unstable_data({...}, responseInit)` utility that will let you send back `status`/`headers` alongside your raw data without having to encode it into a `Response` + * The `headers()` function will let you control header merging for both document and data requests From 8ac568c3aa552cd3edfc64302e56e810e02e4c46 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 14:35:24 -0400 Subject: [PATCH 08/13] Bump router --- integration/package.json | 2 +- packages/remix-dev/package.json | 2 +- packages/remix-react/package.json | 6 +-- packages/remix-server-runtime/package.json | 2 +- packages/remix-testing/package.json | 4 +- pnpm-lock.yaml | 50 +++++++++++----------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/integration/package.json b/integration/package.json index baa3e0a8054..be111b12c70 100644 --- a/integration/package.json +++ b/integration/package.json @@ -14,7 +14,7 @@ "@remix-run/dev": "workspace:*", "@remix-run/express": "workspace:*", "@remix-run/node": "workspace:*", - "@remix-run/router": "0.0.0-experimental-5df42afed", + "@remix-run/router": "0.0.0-experimental-9ffbba722", "@remix-run/server-runtime": "workspace:*", "@types/express": "^4.17.9", "@vanilla-extract/css": "^1.10.0", diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 94ea24dcb1c..c3deccbf41c 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -32,7 +32,7 @@ "@mdx-js/mdx": "^2.3.0", "@npmcli/package-json": "^4.0.1", "@remix-run/node": "workspace:*", - "@remix-run/router": "0.0.0-experimental-5df42afed", + "@remix-run/router": "0.0.0-experimental-9ffbba722", "@remix-run/server-runtime": "workspace:*", "@types/mdx": "^2.0.5", "@vanilla-extract/integration": "^6.2.0", diff --git a/packages/remix-react/package.json b/packages/remix-react/package.json index ea025f0035a..a94277518ed 100644 --- a/packages/remix-react/package.json +++ b/packages/remix-react/package.json @@ -19,10 +19,10 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "0.0.0-experimental-5df42afed", + "@remix-run/router": "0.0.0-experimental-9ffbba722", "@remix-run/server-runtime": "workspace:*", - "react-router": "0.0.0-experimental-5df42afed", - "react-router-dom": "0.0.0-experimental-5df42afed", + "react-router": "0.0.0-experimental-9ffbba722", + "react-router-dom": "0.0.0-experimental-9ffbba722", "turbo-stream": "2.2.0" }, "devDependencies": { diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json index 6ce5fb0d9a3..d2db64f905a 100644 --- a/packages/remix-server-runtime/package.json +++ b/packages/remix-server-runtime/package.json @@ -19,7 +19,7 @@ "tsc": "tsc" }, "dependencies": { - "@remix-run/router": "0.0.0-experimental-5df42afed", + "@remix-run/router": "0.0.0-experimental-9ffbba722", "@types/cookie": "^0.6.0", "@web3-storage/multipart-parser": "^1.0.0", "cookie": "^0.6.0", diff --git a/packages/remix-testing/package.json b/packages/remix-testing/package.json index b2baa0959e1..8b98e838730 100644 --- a/packages/remix-testing/package.json +++ b/packages/remix-testing/package.json @@ -21,8 +21,8 @@ "dependencies": { "@remix-run/node": "workspace:*", "@remix-run/react": "workspace:*", - "@remix-run/router": "0.0.0-experimental-5df42afed", - "react-router-dom": "0.0.0-experimental-5df42afed" + "@remix-run/router": "0.0.0-experimental-9ffbba722", + "react-router-dom": "0.0.0-experimental-9ffbba722" }, "devDependencies": { "@remix-run/server-runtime": "workspace:*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d5d4488bdae..13d6ae3583f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -323,8 +323,8 @@ importers: specifier: workspace:* version: link:../packages/remix-node '@remix-run/router': - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722 '@remix-run/server-runtime': specifier: workspace:* version: link:../packages/remix-server-runtime @@ -871,8 +871,8 @@ importers: specifier: ^2.10.3 version: link:../remix-react '@remix-run/router': - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722 '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime @@ -1217,17 +1217,17 @@ importers: packages/remix-react: dependencies: '@remix-run/router': - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722 '@remix-run/server-runtime': specifier: workspace:* version: link:../remix-server-runtime react-router: - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed(react@18.2.0) + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722(react@18.2.0) react-router-dom: - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0) + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722(react-dom@18.2.0)(react@18.2.0) turbo-stream: specifier: 2.2.0 version: 2.2.0 @@ -1303,8 +1303,8 @@ importers: packages/remix-server-runtime: dependencies: '@remix-run/router': - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722 '@types/cookie': specifier: ^0.6.0 version: 0.6.0 @@ -1340,11 +1340,11 @@ importers: specifier: workspace:* version: link:../remix-react '@remix-run/router': - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722 react-router-dom: - specifier: 0.0.0-experimental-5df42afed - version: 0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0) + specifier: 0.0.0-experimental-9ffbba722 + version: 0.0.0-experimental-9ffbba722(react-dom@18.2.0)(react@18.2.0) devDependencies: '@remix-run/server-runtime': specifier: workspace:* @@ -4201,8 +4201,8 @@ packages: - encoding dev: false - /@remix-run/router@0.0.0-experimental-5df42afed: - resolution: {integrity: sha512-J2cSDEmAxP2gxTi7U4zePG5frP/BKpB6AtUpITTZomFoOoCfz81HzBTJak4NcCsS1jCFgukRIZbBW8IvkESD/Q==} + /@remix-run/router@0.0.0-experimental-9ffbba722: + resolution: {integrity: sha512-35QdzkxjnzP376sr25yrFpNEL3+7ClpRC7DlWYjLr3sBNhce+h1+/9tiop8ILUfcqCbnaPnb1obdqwEas0YmZA==} engines: {node: '>=14.0.0'} dev: false @@ -12786,26 +12786,26 @@ packages: engines: {node: '>=0.10.0'} dev: false - /react-router-dom@0.0.0-experimental-5df42afed(react-dom@18.2.0)(react@18.2.0): - resolution: {integrity: sha512-1B1Qb5Qw+b326WoRfsDXPs3anb45aFvRb8btbpdNBodp1ZkPwS0h1Y5TNcOoo81FvBPeSOydmFmziCpFR58hYg==} + /react-router-dom@0.0.0-experimental-9ffbba722(react-dom@18.2.0)(react@18.2.0): + resolution: {integrity: sha512-ZO9bA0lefVK6jnqHSAolHCXTBYURD8/4GQDT2FHYTQoxu+cpEz2g1l/f3BM0zYG5zAFyIgVDXHXU+QB2zG+NzQ==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' react-dom: '>=16.8' dependencies: - '@remix-run/router': 0.0.0-experimental-5df42afed + '@remix-run/router': 0.0.0-experimental-9ffbba722 react: 18.2.0 react-dom: 18.2.0(react@18.2.0) - react-router: 0.0.0-experimental-5df42afed(react@18.2.0) + react-router: 0.0.0-experimental-9ffbba722(react@18.2.0) dev: false - /react-router@0.0.0-experimental-5df42afed(react@18.2.0): - resolution: {integrity: sha512-GLkDE+AZdgWFwTg4zpAEwZsSHK0ZNvZdkjSqcPkH9fYBJqvu7aOK+hatx6x+7J7HiTij0U1sr8CsUDYMTsTEUw==} + /react-router@0.0.0-experimental-9ffbba722(react@18.2.0): + resolution: {integrity: sha512-Ycu8bo0VQUiMhnbYLCMSkJTUkMivT/Cbt2ff4RUIWfJCXXRCrmTIek8KOzt0ZeYkaoYiP8aQD/Gpxm4m/QgAEA==} engines: {node: '>=14.0.0'} peerDependencies: react: '>=16.8' dependencies: - '@remix-run/router': 0.0.0-experimental-5df42afed + '@remix-run/router': 0.0.0-experimental-9ffbba722 react: 18.2.0 dev: false From 81a346ea35444726b6aac0e944516300852b94d2 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 14:35:39 -0400 Subject: [PATCH 09/13] Proxy error statuses along --- packages/remix-react/single-fetch.tsx | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx index 0c512fc1439..7c2fc2286ba 100644 --- a/packages/remix-react/single-fetch.tsx +++ b/packages/remix-react/single-fetch.tsx @@ -164,15 +164,19 @@ function singleFetchActionStrategy( actionStatus = status; return unwrapSingleFetchResult(data as SingleFetchResult, m.route.id); }); - // If we have a response here, like a redirect, return it as is - if (isResponse(result)) { - return { type: "data", result }; - } - // Otherwise, proxy along the statusCode via unstable_data() - // (most notably for skipping action error revalidation) - return { type: "data", result: unstable_data(result, actionStatus) }; + return { type: "data", result }; }); - return result; + + if (isResponse(result.result)) { + return result; + } + + // For non-responses, proxy along the statusCode via unstable_data() + // (most notably for skipping action error revalidation) + return { + type: result.type, + result: unstable_data(result.result, actionStatus), + }; }) ); } From 11c96b95b7859887e812720ce8a5d1bf8210e86c Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 15:07:41 -0400 Subject: [PATCH 10/13] Fix tests and update changesets --- .changeset/add-unstable-data.md | 9 +++++++++ .../{polite-pumas-visit.md => remove-response-stub.md} | 3 ++- packages/remix-react/single-fetch.tsx | 3 ++- 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 .changeset/add-unstable-data.md rename .changeset/{polite-pumas-visit.md => remove-response-stub.md} (97%) diff --git a/.changeset/add-unstable-data.md b/.changeset/add-unstable-data.md new file mode 100644 index 00000000000..f1eebbd186a --- /dev/null +++ b/.changeset/add-unstable-data.md @@ -0,0 +1,9 @@ +--- +"@remix-run/cloudflare": minor +"@remix-run/deno": minor +"@remix-run/node": minor +"@remix-run/react": minor +"@remix-run/server-runtime": minor +--- + +Add a new `unstable_data()` API for usage with Remix Single Fetch diff --git a/.changeset/polite-pumas-visit.md b/.changeset/remove-response-stub.md similarity index 97% rename from .changeset/polite-pumas-visit.md rename to .changeset/remove-response-stub.md index f1207c6de0d..09b70ccded3 100644 --- a/.changeset/polite-pumas-visit.md +++ b/.changeset/remove-response-stub.md @@ -1,5 +1,6 @@ --- -"@remix-run/server-runtime": patch +"@remix-run/server-runtime": minor +"@remix-run/react": minor --- Single Fetch: Remove `responseStub` in favor of `headers` diff --git a/packages/remix-react/single-fetch.tsx b/packages/remix-react/single-fetch.tsx index 7c2fc2286ba..577c0699a21 100644 --- a/packages/remix-react/single-fetch.tsx +++ b/packages/remix-react/single-fetch.tsx @@ -7,6 +7,7 @@ import type { } from "@remix-run/router"; import { UNSAFE_ErrorResponseImpl as ErrorResponseImpl, + isRouteErrorResponse, redirect, unstable_data, } from "@remix-run/router"; @@ -167,7 +168,7 @@ function singleFetchActionStrategy( return { type: "data", result }; }); - if (isResponse(result.result)) { + if (isResponse(result.result) || isRouteErrorResponse(result.result)) { return result; } From ed5f1d338c278c203a74d05f819647e67aae8adc Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 15:39:06 -0400 Subject: [PATCH 11/13] Remove wrangler change for test debugging --- integration/helpers/vite.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/integration/helpers/vite.ts b/integration/helpers/vite.ts index 32beed34620..65c805288ff 100644 --- a/integration/helpers/vite.ts +++ b/integration/helpers/vite.ts @@ -185,16 +185,7 @@ export const wranglerPagesDev = async ({ let proc = spawn( nodeBin, - [ - wranglerBin, - "pages", - "dev", - "./build/client", - "--port", - String(port), - "--compatibility-date", - "2024-01-29", - ], + [wranglerBin, "pages", "dev", "./build/client", "--port", String(port)], { cwd, stdio: "pipe", From 8ebc974bce31c6002281290438933d5413216336 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jul 2024 13:45:19 -0400 Subject: [PATCH 12/13] Update docs --- docs/guides/single-fetch.md | 134 +++++++----------------------------- docs/utils/data.md | 41 +++++++++++ 2 files changed, 64 insertions(+), 111 deletions(-) create mode 100644 docs/utils/data.md diff --git a/docs/guides/single-fetch.md b/docs/guides/single-fetch.md index 5d8bfa4fec4..884da73d54a 100644 --- a/docs/guides/single-fetch.md +++ b/docs/guides/single-fetch.md @@ -6,7 +6,7 @@ title: Single Fetch This is an unstable API and will continue to change, do not adopt in production -Single fetch is a new data data loading strategy and streaming format. When you enable Single Fetch, Remix will make a single HTTP call to your server on client-side transitions, instead of multiple HTTP calls in parallel (one per loader). Additionally, Single Fetch also allows you to send down naked objects from your `loader` and `action`, such as `Date`, `Error`, `Promise`, `RegExp`, and more. +Single Fetch is a new data data loading strategy and streaming format. When you enable Single Fetch, Remix will make a single HTTP call to your server on client-side transitions, instead of multiple HTTP calls in parallel (one per loader). Additionally, Single Fetch also allows you to send down naked objects from your `loader` and `action`, such as `Date`, `Error`, `Promise`, `RegExp`, and more. ## Overview @@ -51,9 +51,9 @@ Single Fetch requires using [`undici`][undici] as your `fetch` polyfill, or usin - If you are using miniflare/cloudflare worker with your remix project, ensure your [compatibility flag][compatibility-flag] is set to `2023-03-01` or later as well. -**3. Remove document-level `headers` implementation (if you have one)** +**3. Adjust `headers` implementations (if necessary)** -The [`headers`][headers] export is not longer used when single fetch is enabled. In many cases you may have been just re-returning the headers from your loader `Response` instances to apply them to document requests, and if so, you may can likely just remove the export and those Repsonse headers will apply to document requests automatically. If you were doing more complex logic for document headers in the `headers` function, then you will need to migrate those to the new [Response Stub][responsestub] instance in your `loader` functions. +With Single Fetch enabled, there will now only be one request made on client-side navigations even when multiple loaders need to run. To handle merging headers for the handlers called, the [`headers`][headers] export will now also apply to `loader`/`action` data requests. In many cases, the logic you already have in there for document requests should be close to sufficient for your new Single Fetch data requests. **4. Add `nonce` to `` (if you are using a CSP)** @@ -76,7 +76,7 @@ There are a handful of breaking changes introduced with Single Fetch - some of w **Changes that need to be addressed up front:** - **Deprecated `fetch` polyfill**: The old `installGlobals()` polyfill doesn't work for Single Fetch, you must either use the native Node 20 `fetch` API or call `installGlobals({ nativeFetch: true })` in your custom server to get the [undici-based polyfill][undici-polyfill] -- **Deprecated `headers` export**: The [`headers`][headers] function is no longer used when Single Fetch is enabled, in favor of the new `response` stub passed to your `loader`/`action` functions +- **`headers` export applied to data requests**: The [`headers`][headers] function will now apply to both document and data requests **Changes to be aware of that you may need to handle over-time:** @@ -90,7 +90,7 @@ There are a handful of breaking changes introduced with Single Fetch - some of w ## Adding a New Route with Single Fetch -With Single Fetch enabled, you can go ahead and author routes that take advantage of the more powerful streaming format and [`response` stub][responsestub]. +With Single Fetch enabled, you can go ahead and author routes that take advantage of the more powerful streaming format. In order to get proper type inference, you first need to add `@remix-run/react/future/single-fetch.d.ts` to the end of your `tsconfig.json`'s `compilerOptions.types` array. You can read more about this in the [Type Inference section][type-inference-section]. @@ -100,22 +100,18 @@ With Single Fetch you can return the following data types from your loader: `Big // routes/blog.$slug.tsx import { unstable_defineLoader as defineLoader } from "@remix-run/node"; -export const loader = defineLoader( - async ({ params, response }) => { - const { slug } = params; +export const loader = defineLoader(async ({ params }) => { + const { slug } = params; - const comments = fetchComments(slug); - const blogData = await fetchBlogData(slug); + const comments = fetchComments(slug); + const blogData = await fetchBlogData(slug); - response.headers.set("Cache-Control", "max-age=300"); - - return { - content: blogData.content, // <- string - published: blogData.date, // <- Date - comments, // <- Promise - }; - } -); + return { + content: blogData.content, // <- string + published: blogData.date, // <- Date + comments, // <- Promise + }; +}); export default function BlogPost() { const blogData = useLoaderData(); @@ -292,64 +288,18 @@ export default function Component() { ### Headers -The [`headers`][headers] function is no longer used when Single Fetch is enabled. -Instead, your `loader`/`action` functions now receive a mutable `ResponseStub` unique to that execution: +The [`headers`][headers] function is now used on both document and data requests when Single Fetch is enabled. You should use that function to merge any headers returned from loaders executed in parallel, or to return any given `actionHeaders`. -- To alter the status of your HTTP Response, set the `status` field directly: - - `response.status = 201` -- To set the headers on your HTTP Response, use the standard [`Headers`][mdn-headers] APIs: - - `response.headers.set(name, value)` - - `response.headers.append(name, value)` - - `response.headers.delete(name)` +### Returned Responses -```ts -export const action = defineAction( - async ({ request, response }) => { - if (!loggedIn(request)) { - response.status = 401; - response.headers.append("Set-Cookie", "foo=bar"); - return { message: "Invalid Submission!" }; - } - await addItemToDb(request); - return null; - } -); -``` +With Single Fetch, you no longer need to return `Response` instances and can just return your data directly via naked object returns. Therefore, the `json`/`defer` utilities should be considered deprecated when using Single Fetch. These will remain for the duration of v2 so you don't need to remove them immediately. They will likely be removed in the next major version, so we recommend remove them incrementally between now and then. -You can also throw these response stubs to short circuit the flow of your loaders and actions: +For v2, you may still continue returning normal `Response` instances and their `status`/`headers` will take effect the same way they do on document requests (merging headers via the `headers()` function). -```tsx -export const loader = defineLoader( - ({ request, response }) => { - if (shouldRedirectToHome(request)) { - response.status = 302; - response.headers.set("Location", "/"); - throw response; - } - // ... - } -); -``` +Over time, you should start eliminating returned Responses from your loaders and actions. -Each `loader`/`action` receives its own unique `response` instance so you cannot see what other `loader`/`action` functions have set (which would be subject to race conditions). The resulting HTTP Response status and headers are determined as follows: - -- Status Code - - If all status codes are unset or have values <300, the deepest status code will be used for the HTTP response - - If any status codes are set to a value >=300, the shallowest >=300 value will be used for the HTTP Response -- Headers - - Remix tracks header operations and will replay them on a fresh `Headers` instance after all handlers have completed - - These are replayed in order - action first (if present) followed by loaders in top-down order - - `headers.set` on any child handler will overwrite values from parent handlers - - `headers.append` can be used to set the same header from both a parent and child handler - - `headers.delete` can be used to delete a value set by a parent handler, but not a value set from a child handler - -Because Single Fetch supports naked object returns, and you no longer need to return a `Response` instance to set status/headers, the `json`/`redirect`/`redirectDocument`/`defer` utilities should be considered deprecated when using Single Fetch. These will remain for the duration of v2 so you don't need to remove them immediately. They will likely be removed in the next major version, so we recommend remove them incrementally between now and then. - -These utilities will remain for the rest of Remix v2, and it's likely that in a future version they'll be available via something like [`remix-utils`][remix-utils] (or they're also very easy to re-implement yourself). - -For v2, you may still continue returning normal `Response` instances and they'll apply status codes in the same way as the `response` stub, and will apply all headers via `headers.set` - overwriting any same-named header values from parents. If you need to append a header, you will need to switch from returning a `Response` instance to using the new `response` parameter. - -To ensure you can adopt these features incrementally, our goal is that you can enable Single Fetch without changing all of your `loader`/`action` functions to leverage the `response` stub. Then over time, you can incrementally convert individual routes to leverage the new `response` stub. +- If your `loader`/`action` was returning `json`/`defer` without setting any `status`/`headers`, then you can just remove the call to `json`/`defer` and return the data directly +- If you `loader`/`action` was returning custom `status`/`headers` via `json`/`defer`, you should switch those to use the new [`unstable_data()`][data-utility] utility. ### Client Loaders @@ -434,42 +384,6 @@ The Remix v2 behavior with Single Fetch enabled is as follows: Note: It is _not_ recommended to use `defineLoader`/`defineAction` for externally-accessed resource routes that need to return specific `Response` instances. It's best to just stick with `loader`/`LoaderFunctionArgs` for these cases. -#### Response Stub and Resource Routes - -As discussed above, the `headers` export is deprecated in favor of a new [`response` stub][responsestub] passed to your `loader` and `action` functions. This is somewhat confusing in resource routes, though, because you get to return the _actual_ `Response` - there's no real need for a "stub" concept because there's no merging results from multiple loaders into a single Response: - -```tsx filename=app/routes/resource.tsx -// Using your own Response is the most straightforward approach -export async function loader() { - const data = await getData(); - return Response.json(data, { - status: 200, - headers: { - "X-Custom": "whatever", - }, - }); -} -``` - -To keep things consistent, resource route `loader`/`action` functions will still receive a `response` stub and you can use it if you need to (maybe to share code amongst non-resource route handlers): - -```tsx filename=app/routes/resource.tsx -// But you can still set values on the response stub -export async function loader({ - response, -}: LoaderFunctionArgs) { - const data = await getData(); - response?.status = 200; - response?.headers.set("X-Custom", "whatever"); - return Response.json(data); -} -``` - -It's best to try to avoid using the `response` stub _and also_ returning a `Response` with custom status/headers, but if you do, the following logic will apply": - -- The `Response` instance status will take priority over any `response` stub status -- Headers operations on the `response` stub `headers` will be re-played on the returned `Response` headers instance - ## Additional Details ### Streaming Data Format @@ -557,16 +471,13 @@ Revalidation is handled via a `?_routes` query string parameter on the single fe [hydrateroot]: https://react.dev/reference/react-dom/client/hydrateRoot [starttransition]: https://react.dev/reference/react/startTransition [headers]: ../route/headers -[mdn-headers]: https://developer.mozilla.org/en-US/docs/Web/API/Headers [resource-routes]: ../guides/resource-routes [returning-response]: ../route/loader.md#returning-response-instances -[responsestub]: #headers [streaming-format]: #streaming-data-format [undici-polyfill]: https://github.com/remix-run/remix/blob/main/CHANGELOG.md#undici [undici]: https://github.com/nodejs/undici [csp]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src [csp-nonce]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/Sources#sources -[remix-utils]: https://github.com/sergiodxa/remix-utils [merging-remix-and-rr]: https://remix.run/blog/merging-remix-and-react-router [migration-guide]: #migrating-a-route-with-single-fetch [breaking-changes]: #breaking-changes @@ -574,3 +485,4 @@ Revalidation is handled via a `?_routes` query string parameter on the single fe [start]: #enabling-single-fetch [type-inference-section]: #type-inference [compatibility-flag]: https://developers.cloudflare.com/workers/configuration/compatibility-dates +[data-utility]: ../utils/data diff --git a/docs/utils/data.md b/docs/utils/data.md new file mode 100644 index 00000000000..c7872a64372 --- /dev/null +++ b/docs/utils/data.md @@ -0,0 +1,41 @@ +--- +title: unstable_data +toc: false +--- + +# `unstable_data` + +This is a utility for use with [Single Fetch][single-fetch] to return raw data accompanied with a status code or custom response headers. This avoids the need to serialize your data into a `Response` instance to provide custom status/headers. This is generally a replacement for `loader`/`action` functions that used [`json`][json] or [`defer`][defer] prior to Single Fetch. + +```tsx +import { unstable_data as data } from "@remix-run/node"; // or cloudflare/deno + +export const loader = async () => { + // So you can write this: + return data( + { not: "coffee" }, + { + status: 418, + headers: { + "Cache-Control": "no-store", + }, + } + ); +}; +``` + +You should _not_ be using this function if you don't need to return custom status/headers - in that case, just return the data directly: + +```tsx +export const loader = async () => { + // ❌ Bad + return data({ not: "coffee" }); + + // ✅ Good + return { not: "coffee" }; +}; +``` + +[single-fetch]: ../guides/single-fetch +[json]: ./json +[defer]: ./defer From 698ce136227a9346003f92f59c42256571e7ad10 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 25 Jul 2024 13:50:02 -0400 Subject: [PATCH 13/13] typo --- docs/guides/single-fetch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guides/single-fetch.md b/docs/guides/single-fetch.md index 884da73d54a..5571be0d4ec 100644 --- a/docs/guides/single-fetch.md +++ b/docs/guides/single-fetch.md @@ -299,7 +299,7 @@ For v2, you may still continue returning normal `Response` instances and their ` Over time, you should start eliminating returned Responses from your loaders and actions. - If your `loader`/`action` was returning `json`/`defer` without setting any `status`/`headers`, then you can just remove the call to `json`/`defer` and return the data directly -- If you `loader`/`action` was returning custom `status`/`headers` via `json`/`defer`, you should switch those to use the new [`unstable_data()`][data-utility] utility. +- If your `loader`/`action` was returning custom `status`/`headers` via `json`/`defer`, you should switch those to use the new [`unstable_data()`][data-utility] utility. ### Client Loaders