diff --git a/contributors.yml b/contributors.yml index 103b5cab66..2082053191 100644 --- a/contributors.yml +++ b/contributors.yml @@ -26,6 +26,7 @@ - Armanio - arnassavickas - aroyan +- Artur- - ashusnapx - avipatel97 - awreese diff --git a/integration/blocking-test.ts b/integration/blocking-test.ts new file mode 100644 index 0000000000..5b99df29b8 --- /dev/null +++ b/integration/blocking-test.ts @@ -0,0 +1,113 @@ +import { test, expect } from "@playwright/test"; + +import type { AppFixture, Fixture } from "./helpers/create-fixture.js"; +import { + createFixture, + js, + createAppFixture, +} from "./helpers/create-fixture.js"; +import { PlaywrightFixture } from "./helpers/playwright-fixture.js"; + +let fixture: Fixture; +let appFixture: AppFixture; + +test.afterAll(() => appFixture.close()); + +test("handles synchronous proceeding correctly", async ({ page }) => { + fixture = await createFixture({ + files: { + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return ( +
+

Index

+ /a +
+ ) + } + `, + "app/routes/a.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return ( +
+

A

+ /b +
+ ) + } + `, + "app/routes/b.tsx": js` + import * as React from "react"; + import { Form, useAction, useBlocker } from "react-router"; + export default function Component() { + return ( +
+

B

+ +
+ ) + } + function ImportantForm() { + let [value, setValue] = React.useState(""); + let shouldBlock = React.useCallback( + ({ currentLocation, nextLocation }) => + value !== "" && currentLocation.pathname !== nextLocation.pathname, + [value] + ); + let blocker = useBlocker(shouldBlock); + // Reset the blocker if the user cleans the form + React.useEffect(() => { + if (blocker.state === "blocked") { + blocker.proceed(); + } + }, [blocker]); + return ( + <> +

+ Is the form dirty?{" "} + {value !== "" ? ( + Yes + ) : ( + No + )} +

+
+ + +
+ + ); + } + `, + }, + }); + + // This creates an interactive app using puppeteer. + appFixture = await createAppFixture(fixture); + + let app = new PlaywrightFixture(appFixture, page); + + await app.goto("/"); + await app.clickLink("/a"); + await page.waitForSelector("#a"); + await app.clickLink("/b"); + await page.waitForSelector("#b"); + await page.getByLabel("Enter some important data:").fill("Hello Remix!"); + + // Going back should: + // - block + // - immediately call blocker.proceed() once we enter the blocked state + // - and land back one history entry (/a) + await page.goBack(); + await page.waitForSelector("#a"); + expect(await app.getHtml()).toContain("A"); +}); diff --git a/integration/error-boundary-v2-test.ts b/integration/error-boundary-v2-test.ts index 0bce843770..6cb52156bc 100644 --- a/integration/error-boundary-v2-test.ts +++ b/integration/error-boundary-v2-test.ts @@ -177,7 +177,7 @@ test.describe("ErrorBoundary", () => { await waitForAndAssert( page, app, - "#child-error", + "#parent-error", "Unable to decode turbo-stream response" ); }); diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index 652e4f8e61..6dd1888aad 100644 --- a/integration/fog-of-war-test.ts +++ b/integration/fog-of-war-test.ts @@ -1,4 +1,3 @@ -import type { Request as PlaywrightRequest } from "@playwright/test"; import { test, expect } from "@playwright/test"; import { @@ -710,9 +709,7 @@ test.describe("Fog of War", () => { expect(await app.getHtml("#parent")).toMatch(`Parent`); expect(await app.getHtml("#child2")).toMatch(`Child 2`); expect(manifestRequests).toEqual([ - expect.stringMatching( - /\/__manifest\?version=[a-z0-9]{8}&p=%2Fparent%2Fchild2/ - ), + expect.stringMatching(/\/__manifest\?p=%2Fparent%2Fchild2&version=/), ]); }); @@ -780,7 +777,7 @@ test.describe("Fog of War", () => { ) ).toEqual(["root", "routes/_index", "routes/$slug"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fsomething/), + expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), ]); manifestRequests = []; @@ -794,7 +791,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#static"); expect(await app.getHtml("#static")).toMatch("Static"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fstatic/), + expect.stringMatching(/\/__manifest\?p=%2Fstatic&version=/), ]); expect( await page.evaluate(() => @@ -867,7 +864,7 @@ test.describe("Fog of War", () => { ) ).toEqual(["root", "routes/_index", "routes/$"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fsomething/), + expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), ]); manifestRequests = []; @@ -881,7 +878,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#static"); expect(await app.getHtml("#static")).toMatch("Static"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fstatic/), + expect.stringMatching(/\/__manifest\?p=%2Fstatic&version=/), ]); expect( await page.evaluate(() => @@ -953,7 +950,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#slug"); expect(await app.getHtml("#slug")).toMatch("Slug: a"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fa/), + expect.stringMatching(/\/__manifest\?p=%2Fa&version=/), ]); manifestRequests = []; @@ -974,7 +971,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#slug"); expect(await app.getHtml("#slug")).toMatch("Slug: b"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fb/), + expect.stringMatching(/\/__manifest\?p=%2Fb&version=/), ]); }); @@ -1041,7 +1038,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#splat"); expect(await app.getHtml("#splat")).toMatch("Splat: a"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fa/), + expect.stringMatching(/\/__manifest\?p=%2Fa&version=/), ]); manifestRequests = []; @@ -1062,7 +1059,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#splat"); expect(await app.getHtml("#splat")).toMatch("Splat: b/c"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fb%2Fc/), + expect.stringMatching(/\/__manifest\?p=%2Fb%2Fc&version=/), ]); }); @@ -1134,9 +1131,7 @@ test.describe("Fog of War", () => { await app.clickLink("/not/a/path"); await page.waitForSelector("#error"); expect(manifestRequests).toEqual([ - expect.stringMatching( - /\/__manifest\?version=[a-z0-9]{8}&p=%2Fnot%2Fa%2Fpath/ - ), + expect.stringMatching(/\/__manifest\?p=%2Fnot%2Fa%2Fpath&version=/), ]); manifestRequests = []; @@ -1144,7 +1139,7 @@ test.describe("Fog of War", () => { await app.clickLink("/something"); await page.waitForSelector("#slug"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?version=[a-z0-9]{8}&p=%2Fsomething/), + expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), ]); manifestRequests = []; @@ -1177,10 +1172,10 @@ test.describe("Fog of War", () => { let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); - let manifestRequests: PlaywrightRequest[] = []; + let manifestRequests: string[] = []; page.on("request", (req) => { if (req.url().includes("/__manifest")) { - manifestRequests.push(req); + manifestRequests.push(req.url()); } }); @@ -1197,4 +1192,85 @@ test.describe("Fog of War", () => { ) ).toEqual(["root", "routes/_index", "routes/a"]); }); + + test("includes a version query parameter as a cachebuster", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...getFiles(), + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Index() { + return ( + <> +

Index

+ /a + /b + + ); + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let manifestRequests: string[] = []; + page.on("request", (req) => { + if (req.url().includes("/__manifest")) { + manifestRequests.push(req.url()); + } + }); + + await app.goto("/", true); + await new Promise((resolve) => setTimeout(resolve, 250)); + expect(manifestRequests).toEqual([ + expect.stringMatching( + /\/__manifest\?p=%2F&p=%2Fa&p=%2Fb&version=[a-z0-9]{8}/ + ), + ]); + }); + + test("sorts url parameters", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...getFiles(), + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Index() { + return ( + <> +

Index

+ /a + /c + /e + /g + /f + /d + /b + + ); + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let manifestRequests: string[] = []; + page.on("request", (req) => { + if (req.url().includes("/__manifest")) { + manifestRequests.push(req.url()); + } + }); + + await app.goto("/", true); + await new Promise((resolve) => setTimeout(resolve, 250)); + expect(manifestRequests).toEqual([ + expect.stringMatching( + /\/__manifest\?p=%2F&p=%2Fa&p=%2Fb&p=%2Fc&p=%2Fd&p=%2Fe&p=%2Ff&p=%2F/ + ), + ]); + }); }); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index 0d1c253f6f..85cd68bd77 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -115,12 +115,13 @@ export async function createFixture(init: FixtureInit, mode?: ServerMode) { let url = new URL(href, "test://test"); let request = new Request(url.toString(), init); let response = await handler(request); - let decoded = await decodeViaTurboStream(response.body!, global); return { status: response.status, statusText: response.statusText, headers: response.headers, - data: decoded.value, + data: response.body + ? (await decodeViaTurboStream(response.body!, global)).value + : null, }; }; diff --git a/integration/root-route-test.ts b/integration/root-route-test.ts index 13b4b3c035..5326c22a2f 100644 --- a/integration/root-route-test.ts +++ b/integration/root-route-test.ts @@ -249,7 +249,7 @@ test.describe("root route", () => { // which should throw on the initial render _and_ the error render, // resulting in us bubbling to the default error boundary and skipping // our Layout component entirely to avoid a loop - lazy: new Promise((r) => setTimeout(() => r(null), 100)), + lazy: new Promise((r) => setTimeout(() => r(null), 500)), }; } export default function Root() { diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 074959c5e1..a5d8d588ad 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -152,12 +152,9 @@ test.describe("single-fetch", () => { }); test("loads proper data on single fetch loader requests", async () => { - let fixture = await createFixture( - { - files, - }, - ServerMode.Development - ); + let fixture = await createFixture({ + files, + }); let res = await fixture.requestSingleFetchData("/_root.data"); expect(res.data).toEqual({ root: { @@ -169,6 +166,7 @@ test.describe("single-fetch", () => { data: null, }, }); + expect(res.headers.get("Content-Type")).toBe("text/x-script"); res = await fixture.requestSingleFetchData("/data.data"); expect(res.data).toEqual({ @@ -210,12 +208,9 @@ test.describe("single-fetch", () => { }); test("loads proper data on single fetch action requests", async () => { - let fixture = await createFixture( - { - files, - }, - ServerMode.Development - ); + let fixture = await createFixture({ + files, + }); let postBody = new URLSearchParams(); postBody.set("key", "value"); let res = await fixture.requestSingleFetchData("/data.data", { @@ -982,28 +977,25 @@ test.describe("single-fetch", () => { }); test("processes thrown action redirects via Response", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from 'react-router'; - export function action() { - throw redirect('/target'); - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, + let fixture = await createFixture({ + files: { + ...files, + "app/routes/data.tsx": js` + import { redirect } from 'react-router'; + export function action() { + throw redirect('/target'); + } + export default function Component() { + return null + } + `, + "app/routes/target.tsx": js` + export default function Component() { + return

Target

+ } + `, }, - ServerMode.Development - ); + }); console.error = () => {}; @@ -1028,7 +1020,7 @@ test.describe("single-fetch", () => { }); expect(status).toBe(202); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); await app.clickSubmitButton("/data"); @@ -1037,28 +1029,25 @@ test.describe("single-fetch", () => { }); test("processes returned action redirects via Response", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/data.tsx": js` - import { redirect } from 'react-router'; - export function action() { - return redirect('/target'); - } - export default function Component() { - return null - } - `, - "app/routes/target.tsx": js` - export default function Component() { - return

Target

- } - `, - }, + let fixture = await createFixture({ + files: { + ...files, + "app/routes/data.tsx": js` + import { redirect } from 'react-router'; + export function action() { + return redirect('/target'); + } + 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", @@ -1081,7 +1070,7 @@ test.describe("single-fetch", () => { }); expect(status).toBe(202); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); await app.clickSubmitButton("/data"); @@ -1542,157 +1531,243 @@ test.describe("single-fetch", () => { ); }); - test.describe("client loaders", () => { - test("when no routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, - }, - ServerMode.Development - ); + test("Strips ?_routes query param from loader/action requests", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + export function loader({ request }) { + return { url: request.url }; + } + export default function Component() { + return ( + <> +

Parent loader URL: {useLoaderData().url}

+ + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + export function loader({ request }) { + return { url: request.url }; + } + export async function clientLoader({ request, serverLoader }) { + let serverData = await serverLoader(); + return { + serverUrl: serverData.url, + clientUrl: request.url + } + } + export default function Component() { + let data = useLoaderData(); + return ( + <> +

A server loader URL: {data.serverUrl}

+

A client loader URL: {data.clientUrl}

+ + ); + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); - let urls: string[] = []; - page.on("request", (req) => { - if (req.method() === "GET" && req.url().includes(".data")) { - urls.push(req.url()); - } - }); + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/a/b/c"); - await page.waitForSelector("#c-data"); - expect(await app.getHtml("#a-data")).toContain("A server loader"); - expect(await app.getHtml("#b-data")).toContain("B server loader"); - expect(await app.getHtml("#c-data")).toContain("C server loader"); + await app.goto("/"); - // No clientLoaders so we can make a single parameter-less fetch - expect(urls).toEqual([expect.stringMatching(/\/a\/b\/c\.data$/)]); + await app.clickLink("/parent/a"); + await page.waitForSelector("#a-server"); + + // HTTP Requests contained routes params + expect(urls.length).toBe(2); + expect(urls[0].endsWith("/parent/a.data?_routes=routes%2Fparent.a")).toBe( + true + ); + expect( + urls[1].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent") + ).toBe(true); + + // But loaders don't receive any routes params + expect(await app.getHtml("#parent")).toMatch( + />Parent loader URL: http:\/\/localhost:\d+\/parent\/aA server loader URL: http:\/\/localhost:\d+\/parent\/aA client loader URL: http:\/\/localhost:\d+\/parent\/a { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/page.tsx": js` + import { Form, useActionData, useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export function action({ request }) { + return { message: "ACTION" }; + } + export default function Component() { + let data = useLoaderData(); + let actionData = useActionData(); + return ( + <> +

{"Count:" + data.count}

+
+ + {actionData ?

{actionData.message}

: null} +
+ + ) + } + `, + }, }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); - test("when one route has a client loader", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.method() + " " + req.url()); + } + }); + + await app.goto("/page"); + expect(await app.getHtml("#data")).toContain("Count:1"); + + await app.clickSubmitButton("/page"); + await page.waitForSelector("#action"); + expect(await app.getHtml("#data")).toContain("Count:2"); + + // HTTP Requests contained routes params + expect(urls).toEqual([ + expect.stringMatching(/POST .*\/page.data$/), + expect.stringMatching(/GET .*\/page.data$/), + ]); + }); + + test("does not try to encode a turbo-stream body into 304 responses", async () => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { useLoaderData } from "react-router"; + + const eTag = "1234"; + export function loader({ request }) { + if (request.headers.get("If-None-Match") === eTag) { + throw new Response(null, { status: 304 }); + } + return { message: "Hello from the loader!" }; + }; + export default function Index() { + const { message } = useLoaderData(); + return

{message}

+ } + `, + }, + }); + + // Document requests + let documentRes = await fixture.requestDocument("/"); + let html = await documentRes.text(); + expect(html).toContain(""); + expect(html).toContain("

Hello from the loader!

"); + documentRes = await fixture.requestDocument("/", { + headers: { + "If-None-Match": "1234", + }, + }); + expect(documentRes.status).toBe(304); + expect(await documentRes.text()).toBe(""); + + // Data requests + let dataRes = await fixture.requestSingleFetchData("/_root.data"); + expect(dataRes.data).toEqual({ + root: { + data: { + message: "ROOT", }, - ServerMode.Development - ); + }, + "routes/_index": { + data: { + message: "Hello from the loader!", + }, + }, + }); + dataRes = await fixture.requestSingleFetchData("/_root.data", { + headers: { + "If-None-Match": "1234", + }, + }); + expect(dataRes.status).toBe(304); + expect(dataRes.data).toBeNull(); + }); + + test.describe("revalidations/_routes param", () => { + test("does not make a server call if no loaders need to run", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + "app/root.tsx": js` + import { Link, Links, Meta, Outlet, Scripts } from "react-router"; + export default function Root() { + return ( + + + + + + + Home
+ /a/b
+ + + + + ); + } + `, + "app/routes/a.tsx": js` + import { Outlet } from "react-router"; + export default function Root() { + return ; + } + `, + "app/routes/a.b.tsx": js` + export default function Root() { + return

B

; + } + `, + }, + }); let urls: string[] = []; page.on("request", (req) => { @@ -1701,107 +1776,635 @@ test.describe("single-fetch", () => { } }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); - await app.clickLink("/a/b/c"); - await page.waitForSelector("#c-data"); - expect(await app.getHtml("#a-data")).toContain("A server loader"); - expect(await app.getHtml("#b-data")).toContain("B server loader"); - expect(await app.getHtml("#c-data")).toContain( - "C server loader (C client loader)" - ); - // A/B can be loaded together, C needs it's own call due to it's clientLoader - expect(urls.sort()).toEqual([ - expect.stringMatching( - /\/a\/b\/c\.data\?_routes=routes%2Fa%2Croutes%2Fa\.b$/ - ), - expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/), - ]); + await app.clickLink("/a/b"); + await page.waitForSelector("h1"); + expect(await app.getHtml("h1")).toBe("

B

"); + expect(urls.length).toBe(0); }); - test("when multiple routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (B client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + test("calls reused parent routes by default", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return ( + <> +

Parent Count: {useLoaderData().count}

+ Go to /parent/a + Go to /parent/b + + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

A Count: {useLoaderData().count}

; + } + `, + "app/routes/parent.b.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

B Count: {useLoaderData().count}

; + } + `, }, - ServerMode.Development - ); + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); let urls: string[] = []; page.on("request", (req) => { - if (req.method() === "GET" && req.url().includes(".data")) { + if (req.url().includes(".data")) { urls.push(req.url()); } }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + await app.goto("/"); + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 2"); + expect(await app.getHtml("#b")).toContain("B Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/b.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 3"); + expect(await app.getHtml("#a")).toContain("A Count: 2"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + }); + + test("allows reused routes to opt out via shouldRevalidate", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export function shouldRevalidate() { + return false; + } + export default function Component() { + return ( + <> +

Parent Count: {useLoaderData().count}

+ Go to /parent/a + Go to /parent/b + + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

A Count: {useLoaderData().count}

; + } + `, + "app/routes/parent.b.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

B Count: {useLoaderData().count}

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + await app.goto("/"); + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 1"); + expect(urls.length).toBe(1); + // Not a revalidation on the first navigation so no params + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#b")).toContain("B Count: 1"); + expect(urls.length).toBe(1); + // Don't reload the parent route + expect( + urls[0].endsWith("/parent/b.data?_routes=root%2Croutes%2Fparent.b") + ).toBe(true); + urls = []; + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 2"); + expect(urls.length).toBe(1); + // Don't reload the parent route + expect( + urls[0].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a") + ).toBe(true); + }); + + test("allows reused routes to opt out via shouldRevalidate (w/clientLoader)", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export function shouldRevalidate() { + return false; + } + export function clientLoader({ serverLoader }) { + return serverLoader() + } + export default function Component() { + return ( + <> +

Parent Count: {useLoaderData().count}

+ Go to /parent/a + Go to /parent/b + + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

A Count: {useLoaderData().count}

; + } + `, + "app/routes/parent.b.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

B Count: {useLoaderData().count}

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + await app.goto("/"); + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 1"); + expect(urls.length).toBe(2); + // Client loader triggers 2 requests on the first navigation + expect(urls[0].endsWith("/parent/a.data?_routes=routes%2Fparent")).toBe( + true + ); + expect( + urls[1].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a") + ).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#b")).toContain("B Count: 1"); + expect(urls.length).toBe(1); + // Don't reload the parent route + expect( + urls[0].endsWith("/parent/b.data?_routes=root%2Croutes%2Fparent.b") + ).toBe(true); + urls = []; + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 2"); + expect(urls.length).toBe(1); + // Don't reload the parent route + expect( + urls[0].endsWith("/parent/a.data?_routes=root%2Croutes%2Fparent.a") + ).toBe(true); + }); + + test("does not add a _routes param for routes without loaders", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export function shouldRevalidate() { + return false; + } + export default function Component() { + return ( + <> +

Parent Count: {useLoaderData().count}

+ Go to /parent/a + Go to /parent/b + + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

A Count: {useLoaderData().count}

; + } + `, + "app/routes/parent.b.tsx": js` + export default function Component() { + return

B

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + await app.goto("/"); + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 1"); + expect(urls.length).toBe(1); + // Not a revalidation on the first navigation so no params + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#b")).toContain("B"); + expect(urls.length).toBe(1); + // Don't reload the parent route + expect(urls[0].endsWith("/parent/b.data?_routes=root")).toBe(true); + urls = []; + }); + }); + + test.describe("client loaders", () => { + test("when no routes have client loaders", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickLink("/a/b/c"); + await page.waitForSelector("#c-data"); + expect(await app.getHtml("#a-data")).toContain("A server loader"); + expect(await app.getHtml("#b-data")).toContain("B server loader"); + expect(await app.getHtml("#c-data")).toContain("C server loader"); + + // No clientLoaders so we can make a single parameter-less fetch + expect(urls).toEqual([expect.stringMatching(/\/a\/b\/c\.data$/)]); + }); + + test("when one route has a client loader", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickLink("/a/b/c"); + await page.waitForSelector("#c-data"); + expect(await app.getHtml("#a-data")).toContain("A server loader"); + expect(await app.getHtml("#b-data")).toContain("B server loader"); + expect(await app.getHtml("#c-data")).toContain( + "C server loader (C client loader)" + ); + + // root/A/B can be loaded together, C needs it's own call due to it's clientLoader + expect(urls.sort()).toEqual([ + expect.stringMatching( + /\/a\/b\/c\.data\?_routes=root%2Croutes%2Fa%2Croutes%2Fa\.b$/ + ), + expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/), + ]); + }); + + test("when multiple routes have client loaders", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (B client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); await app.clickLink("/a/b/c"); @@ -1814,200 +2417,439 @@ test.describe("single-fetch", () => { "C server loader (C client loader)" ); - // B/C have client loaders so they get individual calls, which leaves A - // getting it's own "individual" since it's the last route standing - expect(urls.sort()).toEqual([ - expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa$/), - expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b$/), - expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/), - ]); + // B/C have client loaders so they get individual calls, root/A go together + expect(urls.sort()).toEqual([ + expect.stringMatching(/\/a\/b\/c\.data\?_routes=root%2Croutes%2Fa$/), + expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b$/), + expect.stringMatching(/\/a\/b\/c\.data\?_routes=routes%2Fa\.b\.c$/), + ]); + }); + + test("when all routes have client loaders", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (A client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (B client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await app.clickLink("/a/b/c"); + await page.waitForSelector("#c-data"); + expect(await app.getHtml("#a-data")).toContain( + "A server loader (A client loader)" + ); + expect(await app.getHtml("#b-data")).toContain( + "B server loader (B client loader)" + ); + expect(await app.getHtml("#c-data")).toContain( + "C server loader (C client loader)" + ); + + // root/A/B/C all have client loaders so they get individual calls + expect(urls.sort()).toEqual([ + expect.stringMatching(/\/a\/b\/c.data\?_routes=root$/), + expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa$/), + expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b$/), + expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b.c$/), + ]); + }); + }); + + test.describe("fetchers", () => { + test("Fetcher loaders call singular routes", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet } from "react-router"; + export default function Comp() { + return ; + } + `, + "app/routes/a.b.tsx": js` + import { useFetcher } from "react-router"; + export function loader() { + return { message: 'LOADER' }; + } + export default function Comp() { + let fetcher = useFetcher(); + return ( + <> + + {fetcher.data ?

{fetcher.data.message}

: null} + + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/a/b"); + await app.clickElement("#load"); + await page.waitForSelector("#data"); + expect(await app.getHtml("#data")).toContain("LOADER"); + + // No clientLoaders so we can make a single parameter-less fetch + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/a/b.data?_routes=routes%2Fa.b")).toBe(true); + }); + + test("Fetcher actions call singular routes", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Outlet } from "react-router"; + export default function Comp() { + return ; + } + `, + "app/routes/a.b.tsx": js` + import { useFetcher } from "react-router"; + export function action() { + return { message: 'ACTION' }; + } + export default function Comp() { + let fetcher = useFetcher(); + return ( + <> + + {fetcher.data ?

{fetcher.data.message}

: null} + + ); + } + `, + }, + }); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.method() === "GET" && req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/a/b"); + await app.clickElement("#submit"); + await page.waitForSelector("#data"); + expect(await app.getHtml("#data")).toContain("ACTION"); + + // No clientLoaders so we can make a single parameter-less fetch + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/a/b.data")).toBe(true); }); - test("when all routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (A client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (B client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + test("Fetcher loads do not revalidate on GET navigations by default", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/parent.tsx": js` + import { Link, Outlet, useFetcher } from "react-router"; + export default function Component() { + let fetcher = useFetcher(); + return ( + <> + Go to /parent/a + Go to /parent/b + + {fetcher.data ?

Fetch Count: {fetcher.data.count}

: null} + + + ); + } + `, + "app/routes/parent.a.tsx": js` + export default function Component() { + return

A

; + } + `, + "app/routes/parent.b.tsx": js` + export default function Component() { + return

B

; + } + `, + "app/routes/fetch.tsx": js` + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

Fetch

; + } + `, }, - ServerMode.Development - ); + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); let urls: string[] = []; page.on("request", (req) => { - if (req.method() === "GET" && req.url().includes(".data")) { + if (req.url().includes(".data")) { urls.push(req.url()); } }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + await app.goto("/parent/a"); + await app.clickElement("#load"); + await page.waitForSelector("#fetch"); + expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/b.data")).toBe(true); + }); + + test("Fetcher loads can opt into revalidation on GET navigations", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/parent.tsx": js` + import { Link, Outlet, useFetcher } from "react-router"; + export default function Component() { + let fetcher = useFetcher(); + return ( + <> + Go to /parent/a + Go to /parent/b + + {fetcher.data ?

Fetch Count: {fetcher.data.count}

: null} + + + ); + } + `, + "app/routes/parent.a.tsx": js` + export default function Component() { + return

A

; + } + `, + "app/routes/parent.b.tsx": js` + export default function Component() { + return

B

; + } + `, + "app/routes/fetch.tsx": js` + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export function shouldRevalidate() { + return true; + } + export default function Component() { + return

Fetch

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - await app.clickLink("/a/b/c"); - await page.waitForSelector("#c-data"); - expect(await app.getHtml("#a-data")).toContain( - "A server loader (A client loader)" - ); - expect(await app.getHtml("#b-data")).toContain( - "B server loader (B client loader)" - ); - expect(await app.getHtml("#c-data")).toContain( - "C server loader (C client loader)" - ); - // A/B/C all have client loaders so they get individual calls - expect(urls.sort()).toEqual([ - expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa$/), - expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b$/), - expect.stringMatching(/\/a\/b\/c.data\?_routes=routes%2Fa.b.c$/), - ]); + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + await app.goto("/parent/a"); + await app.clickElement("#load"); + await page.waitForSelector("#fetch"); + expect(await app.getHtml("#fetch")).toContain("Fetch Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#fetch")).toContain("Fetch Count: 2"); + expect(urls.length).toBe(2); + expect(urls[0].endsWith("/fetch.data?_routes=routes%2Ffetch")).toBe(true); + expect(urls[1].endsWith("/parent/b.data")).toBe(true); }); }); test.describe("prefetching", () => { test("when no routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/_index.tsx": js` - import { Link } from "react-router"; - - export default function Index() { - return ( - - ); - } - `, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + + export default function Index() { + return ( + + ); + } + `, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, }, - ServerMode.Development - ); + }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/", true); // No clientLoaders so we can make a single parameter-less fetch @@ -2019,287 +2861,454 @@ test.describe("single-fetch", () => { }); test("when one route has a client loader", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/_index.tsx": js` - import { Link } from "react-router"; - - export default function Index() { - return ( - - ); - } - `, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + + export default function Index() { + return ( + + ); + } + `, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, }, - ServerMode.Development - ); + }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/", true); - // A/B can be prefetched, C doesn't get prefetched due to its `clientLoader` + // root/A/B can be prefetched, C doesn't get prefetched due to its `clientLoader` await page.waitForSelector( - "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=routes%2Fa%2Croutes%2Fa.b']", + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa%2Croutes%2Fa.b']", { state: "attached" } ); expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); }); test("when multiple routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/_index.tsx": js` - import { Link } from "react-router"; - - export default function Index() { - return ( - - ); - } - `, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (B client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + + export default function Index() { + return ( + + ); + } + `, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (B client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, }, - ServerMode.Development - ); + }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); await app.goto("/", true); - // Only A can get prefetched, B/C can't due to `clientLoader` + // root/A can get prefetched, B/C can't due to `clientLoader` await page.waitForSelector( - "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=routes%2Fa']", + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa']", { state: "attached" } ); expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); }); test("when all routes have client loaders", async ({ page }) => { - let fixture = await createFixture( - { - files: { - ...files, - "app/routes/_index.tsx": js` - import { Link } from "react-router"; - - export default function Index() { - return ( + let fixture = await createFixture({ + files: { + ...files, + "app/root.tsx": js` + import { Links, Meta, Outlet, Scripts } from "react-router"; + export function loader() { + return { + message: "ROOT", + }; + } + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (root client loader)" }; + } + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + + export default function Index() { + return ( + + ); + } + `, + "app/routes/a.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "A server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (A client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + + export function loader() { + return { message: "B server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (B client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/", true); + + // No prefetching due to clientLoaders + expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(0); + }); + + test("when a reused route opts out of revalidation", async ({ page }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Link, Outlet, useLoaderData } from 'react-router'; + export function loader() { + return { message: "A server loader" }; + } + export function shouldRevalidate() { + return false; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

- ); - } - `, - "app/routes/a.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "A server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (A client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

A

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - - export function loader() { - return { message: "B server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (B client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

B

-

{data.message}

- - - ); - } - `, - "app/routes/a.b.c.tsx": js` - import { useLoaderData } from 'react-router'; - - export function loader() { - return { message: "C server loader" }; - } - - export async function clientLoader({ serverLoader }) { - let data = await serverLoader(); - return { message: data.message + " (C client loader)" }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

C

-

{data.message}

- - ); - } - `, - }, + + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + export function loader() { + return { message: "B server loader" }; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + export function loader() { + return { message: "C server loader" }; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, }, - ServerMode.Development + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/a", true); + + // A opted out of revalidation + await page.waitForSelector( + "link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa.b%2Croutes%2Fa.b.c']", + { state: "attached" } ); + expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); + }); + + test("when a reused route opts out of revalidation and another route has a clientLoader", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/a.tsx": js` + import { Link, Outlet, useLoaderData } from 'react-router'; + export function loader() { + return { message: "A server loader" }; + } + export function shouldRevalidate() { + return false; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from 'react-router'; + export function loader() { + return { message: "B server loader" }; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from 'react-router'; + export function loader() { + return { message: "C server loader" }; + } + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); - let appFixture = await createAppFixture(fixture, ServerMode.Development); + let appFixture = await createAppFixture(fixture); let app = new PlaywrightFixture(appFixture, page); - await app.goto("/", true); + await app.goto("/a", true); - // No prefetching due to clientLoaders - expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(0); + // A opted out of revalidation + await page.waitForSelector( + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa.b']", + { state: "attached" } + ); + expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); }); }); diff --git a/packages/react-router-dev/vite/plugin.ts b/packages/react-router-dev/vite/plugin.ts index 27a6a0afe8..cad3d08f65 100644 --- a/packages/react-router-dev/vite/plugin.ts +++ b/packages/react-router-dev/vite/plugin.ts @@ -265,13 +265,13 @@ function resolveDependantChunks( return; } + chunks.add(chunk); + if (chunk.imports) { for (let importKey of chunk.imports) { walk(viteManifest[importKey]); } } - - chunks.add(chunk); } for (let entryChunk of entryChunks) { diff --git a/packages/react-router-dev/vite/static/refresh-utils.cjs b/packages/react-router-dev/vite/static/refresh-utils.cjs index ce6e600c46..d84aeca622 100644 --- a/packages/react-router-dev/vite/static/refresh-utils.cjs +++ b/packages/react-router-dev/vite/static/refresh-utils.cjs @@ -60,7 +60,13 @@ const enqueueUpdate = debounce(async () => { window.__reactRouterRouteModuleUpdates.clear(); } - await revalidate(); + try { + window.__remixHdrActive = true; + await __remixRouter.revalidate(); + } finally { + window.__remixHdrActive = false; + } + if (manifest) { Object.assign(window.__remixManifest, manifest); } @@ -145,31 +151,6 @@ function __hmr_import(module) { const routeUpdates = new Map(); window.__reactRouterRouteModuleUpdates = new Map(); -async function revalidate() { - let { promise, resolve } = channel(); - let unsub = __remixRouter.subscribe((state) => { - if (state.revalidation === "idle") { - unsub(); - // Ensure RouterProvider setState has flushed before re-rendering - resolve(); - } - }); - window.__remixRevalidation = (window.__remixRevalidation || 0) + 1; - __remixRouter.revalidate(); - return promise; -} - -function channel() { - let resolve; - let reject; - - let promise = new Promise((_resolve, _reject) => { - resolve = _resolve; - reject = _reject; - }); - return { promise, resolve, reject }; -} - import.meta.hot.on("react-router:hmr", async ({ route }) => { window.__remixClearCriticalCss(); diff --git a/packages/react-router/__tests__/dom/use-blocker-test.tsx b/packages/react-router/__tests__/dom/use-blocker-test.tsx index f62481a009..5db0e10aca 100644 --- a/packages/react-router/__tests__/dom/use-blocker-test.tsx +++ b/packages/react-router/__tests__/dom/use-blocker-test.tsx @@ -15,10 +15,10 @@ import { type Router = ReturnType; -const LOADER_LATENCY_MS = 100; +const LOADER_LATENCY_MS = 200; async function slowLoader() { - await sleep(LOADER_LATENCY_MS); + await sleep(LOADER_LATENCY_MS / 2); return json(null); } @@ -1084,14 +1084,23 @@ describe("navigation blocking with useBlocker", () => { act(() => { click(node.querySelector("[data-action='back']")); }); - act(() => { + expect(node.innerHTML).toContain("

Contact

"); + await act(async () => { click(node.querySelector("[data-action='proceed']")); + expect([...router.state.blockers.values()][0]).toEqual({ + state: "proceeding", + proceed: undefined, + reset: undefined, + location: expect.any(Object), + }); + await sleep(LOADER_LATENCY_MS); }); + expect(node.innerHTML).toContain("

About

"); expect(blocker).toEqual({ - state: "proceeding", + state: "unblocked", proceed: undefined, reset: undefined, - location: expect.any(Object), + location: undefined, }); }); diff --git a/packages/react-router/__tests__/router/data-strategy-test.ts b/packages/react-router/__tests__/router/data-strategy-test.ts index 488fca46f2..501981ac62 100644 --- a/packages/react-router/__tests__/router/data-strategy-test.ts +++ b/packages/react-router/__tests__/router/data-strategy-test.ts @@ -1,6 +1,7 @@ import type { DataStrategyFunction, DataStrategyMatch, + DataStrategyResult, } from "../../lib/router/utils"; import { json } from "../../lib/router/utils"; import { createDeferred, setup } from "./utils/data-router-setup"; @@ -14,10 +15,26 @@ describe("router dataStrategy", () => { >(fn); } + function keyedResults( + matches: DataStrategyMatch[], + results: DataStrategyResult[] + ) { + return results.reduce( + (acc, r, i) => + Object.assign( + acc, + matches[i].shouldLoad ? { [matches[i].route.id]: r } : {} + ), + {} + ); + } + describe("loaders", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -75,7 +92,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -145,13 +164,10 @@ describe("router dataStrategy", () => { matches.map((m) => m.resolve(async (handler) => { let result = await handler(); - return { - type: "data", - result: `Route ID "${m.route.id}" returned "${result}"`, - }; + return `Route ID "${m.route.id}" returned "${result}"`; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -163,6 +179,38 @@ describe("router dataStrategy", () => { }); }); + it("should allow custom implementations to override default behavior when erroring", async () => { + let t = setup({ + routes: [ + { + path: "/", + }, + { + id: "test", + path: "/test", + loader: true, + hasErrorBoundary: true, + }, + ], + async dataStrategy({ matches }) { + return Promise.all( + matches.map((m) => + m.resolve(async () => { + throw new Error(`Route ID "${m.route.id}" errored!`); + }) + ) + ).then((results) => keyedResults(matches, results)); + }, + }); + + let A = await t.navigate("/test"); + await A.loaders.test.resolve("TEST"); + + expect(t.router.state.errors).toMatchObject({ + test: new Error('Route ID "test" errored!'), + }); + }); + it("should allow custom implementations to override default behavior with lazy", async () => { let t = setup({ routes: [ @@ -180,13 +228,10 @@ describe("router dataStrategy", () => { matches.map((m) => m.resolve(async (handler) => { let result = await handler(); - return { - type: "data", - result: `Route ID "${m.route.id}" returned "${result}"`, - }; + return `Route ID "${m.route.id}" returned "${result}"`; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -223,7 +268,9 @@ describe("router dataStrategy", () => { }, ], dataStrategy({ matches }) { - return Promise.all(matches.map(async (match) => match.resolve())); + return Promise.all( + matches.map(async (match) => match.resolve()) + ).then((results) => keyedResults(matches, results)); }, }); @@ -268,7 +315,7 @@ describe("router dataStrategy", () => { }; }); }) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -318,7 +365,7 @@ describe("router dataStrategy", () => { }); }); - it("throws an error if an implementation does not call resolve", async () => { + it("does not require resolve to be called if a match is not being loaded", async () => { let t = setup({ routes: [ { @@ -338,35 +385,47 @@ describe("router dataStrategy", () => { ], }, ], - // @ts-expect-error - dataStrategy({ matches }) { + dataStrategy({ matches, request }) { return Promise.all( matches.map(async (match) => { - if (match.route.id === "child") { - // noop to cause error - return "forgot to load child"; + if ( + request.url.endsWith("/parent/child") && + match.route.id === "parent" + ) { + return undefined; } return match.resolve(); }) + ).then((results) => + // @ts-expect-error + keyedResults(matches, results) ); }, }); - let A = await t.navigate("/parent/child"); + let A = await t.navigate("/parent"); await A.loaders.parent.resolve("PARENT"); - expect(t.router.state).toMatchObject({ - actionData: null, - errors: { - parent: new Error( - '`match.resolve()` was not called for route id "child". ' + - "You must call `match.resolve()` on every match passed " + - "to `dataStrategy` to ensure all routes are properly loaded." - ), + errors: null, + loaderData: { + parent: "PARENT", + }, + navigation: { + state: "idle", }, + }); + + let B = await t.navigate("/parent/child"); + await B.lazy.child.resolve({ loader: () => "CHILD" }); + + // no-op + await B.loaders.parent.resolve("XXX"); + + expect(t.router.state).toMatchObject({ + errors: null, loaderData: { - child: undefined, - parent: undefined, + child: "CHILD", + parent: "PARENT", }, navigation: { state: "idle", @@ -379,7 +438,9 @@ describe("router dataStrategy", () => { ReturnType, Parameters >(({ matches }) => { - return Promise.all(matches.map((m) => m.resolve())); + return Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ); }); let t = setup({ routes: [ @@ -513,7 +574,9 @@ describe("router dataStrategy", () => { describe("actions", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -555,7 +618,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -600,7 +665,9 @@ describe("router dataStrategy", () => { describe("loaders", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -639,7 +706,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -679,7 +748,9 @@ describe("router dataStrategy", () => { describe("actions", () => { it("should allow a custom implementation to passthrough to default behavior", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -721,7 +792,9 @@ describe("router dataStrategy", () => { it("should allow a custom implementation to passthrough to default behavior and lazy", async () => { let dataStrategy = mockDataStrategy(({ matches }) => - Promise.all(matches.map((m) => m.resolve())) + Promise.all(matches.map((m) => m.resolve())).then((results) => + keyedResults(matches, results) + ) ); let t = setup({ routes: [ @@ -795,18 +868,15 @@ describe("router dataStrategy", () => { ) { let str = await result.text(); return { - type: "data", - result: { - original: str, - reversed: str.split("").reverse().join(""), - }, + original: str, + reversed: str.split("").reverse().join(""), }; } // This will be a JSON response we expect to be decoded the normal way - return { type: "data", result }; + return result; }); }) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -827,6 +897,7 @@ describe("router dataStrategy", () => { }); }); + jest.setTimeout(10000000); it("allows a single-fetch type approach", async () => { let t = setup({ routes: [ @@ -861,7 +932,7 @@ describe("router dataStrategy", () => { // the single fetch response and return it's promise let dfd = createDeferred(); routeDeferreds.set(m.route.id, dfd); - return dfd.promise; + return dfd.promise as Promise; }) ); @@ -871,14 +942,15 @@ describe("router dataStrategy", () => { parent: "PARENT", child: "CHILD", }, - errors: null, }; // Resolve the deferred's above and return the mapped match promises routeDeferreds.forEach((dfd, routeId) => - dfd.resolve({ type: "data", result: result.loaderData[routeId] }) + dfd.resolve(result.loaderData[routeId]) + ); + return Promise.all(matchPromises).then((results) => + keyedResults(matches, results) ); - return Promise.all(matchPromises); }, }); @@ -960,10 +1032,11 @@ describe("router dataStrategy", () => { }); return acc; }, {}); - return { type: "data", result: await handler(handlerCtx) }; + let result = await handler(handlerCtx); + return result; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -1079,10 +1152,13 @@ describe("router dataStrategy", () => { }); return acc; }, {}); - return { type: "data", result: await callHandler(handlerCtx) }; + let result = m.shouldLoad + ? await callHandler(handlerCtx) + : t.router.state.loaderData[m.route.id]; + return result; }) ) - ); + ).then((results) => keyedResults(matches, results)); }, }); @@ -1153,29 +1229,29 @@ describe("router dataStrategy", () => { ? [m.route.id, m.route.handle.cacheKey(request.url)].join("-") : null; + if (request.method !== "GET") { + // invalidate on actions + cache = {}; + } + + let matchesToLoad = matches.filter((m) => m.shouldLoad); return Promise.all( - matches.map(async (m) => { + matchesToLoad.map(async (m) => { return m.resolve(async (handler) => { - if (request.method !== "GET") { - // invalidate on actions - cache = {}; - return { type: "data", result: await handler() }; - } - let key = getCacheKey(m); if (key && cache[key]) { - return { type: "data", result: cache[key] }; + return cache[key]; } - let handlerResult = await handler(); - if (key) { - cache[key] = handlerResult; + let dsResult = await handler(); + if (key && request.method === "GET") { + cache[key] = dsResult; } - return { type: "data", result: handlerResult }; + return dsResult; }); }) - ); + ).then((results) => keyedResults(matchesToLoad, results)); }, }); diff --git a/packages/react-router/__tests__/router/fetchers-test.ts b/packages/react-router/__tests__/router/fetchers-test.ts index dc022a0318..c325671f22 100644 --- a/packages/react-router/__tests__/router/fetchers-test.ts +++ b/packages/react-router/__tests__/router/fetchers-test.ts @@ -15,7 +15,7 @@ import { setup, TASK_ROUTES, } from "./utils/data-router-setup"; -import { createFormData, tick } from "./utils/utils"; +import { createFormData, sleep, tick } from "./utils/utils"; function initializeTest(init?: { url?: string; @@ -2307,7 +2307,7 @@ describe("fetchers", () => { loader: true, shouldRevalidate: () => false, }, - // fetch C will not before the action, and will not be able to opt + // fetch C will not resolve before the action, and will not be able to opt // out because it has no data { id: "fetchC", @@ -2504,6 +2504,69 @@ describe("fetchers", () => { expect(C.loaders.fetch.stub).not.toHaveBeenCalled(); }); + // This is another example of the above bug where cancelled fetchers were not + // cleaned up correctly (https://github.com/remix-run/remix/issues/8298). + // It was also fixed by https://github.com/remix-run/react-router/pull/11839 + it("Remix Github Issue 8298", async () => { + let loaderCount = 0; + let router = createRouter({ + history: createMemoryHistory(), + routes: [ + { + id: "index", + path: "/", + }, + { + id: "loader", + path: "/loader", + async loader() { + let count = ++loaderCount; + await sleep(100); + return count; + }, + }, + { + id: "action", + path: "/action", + async action() { + await sleep(100); + return "ACTION"; + }, + }, + ], + }); + router.initialize(); + + let fetcherData = new Map(); + router.subscribe((state) => { + state.fetchers.forEach((fetcher, key) => { + fetcherData.set(key, fetcher.data); + }); + }); + + router.fetch("a", "index", "/loader"); + expect(router.getFetcher("a")).toMatchObject({ state: "loading" }); + + await sleep(250); + router.revalidate(); + + await sleep(250); + router.fetch("b", "index", "/action", { + formMethod: "post", + body: createFormData({}), + }); + expect(router.getFetcher("b")).toMatchObject({ state: "submitting" }); + + await sleep(250); + + expect(router.getFetcher("b")).toMatchObject({ state: "idle" }); + expect(fetcherData.get("b")).toBe("ACTION"); + + // fetcher load, router revalidation, action revalidation + expect(router.getFetcher("a")).toMatchObject({ state: "idle" }); + expect(fetcherData.get("a")).toBe(3); + }); + it("does not cancel pending action navigation on deletion of revalidating fetcher", async () => { let t = setup({ routes: TASK_ROUTES, diff --git a/packages/react-router/__tests__/router/navigation-blocking-test.ts b/packages/react-router/__tests__/router/navigation-blocking-test.ts index 7f7a603e38..a963b26e8c 100644 --- a/packages/react-router/__tests__/router/navigation-blocking-test.ts +++ b/packages/react-router/__tests__/router/navigation-blocking-test.ts @@ -443,7 +443,7 @@ describe("navigation blocking", () => { router.getBlocker("KEY", fn); await router.navigate(-1); router.getBlocker("KEY", fn).proceed?.(); - await sleep(LOADER_LATENCY_MS); + await sleep(LOADER_LATENCY_MS + 10); expect(router.getBlocker("KEY", fn)).toEqual({ state: "unblocked", proceed: undefined, @@ -456,7 +456,7 @@ describe("navigation blocking", () => { router.getBlocker("KEY", fn); await router.navigate(-1); router.getBlocker("KEY", fn).proceed?.(); - await sleep(LOADER_LATENCY_MS); + await sleep(LOADER_LATENCY_MS + 10); expect(router.state.location.pathname).toBe("/about"); }); }); diff --git a/packages/react-router/__tests__/router/utils/urlDataStrategy.ts b/packages/react-router/__tests__/router/utils/urlDataStrategy.ts index b561c51ba7..157633d262 100644 --- a/packages/react-router/__tests__/router/utils/urlDataStrategy.ts +++ b/packages/react-router/__tests__/router/utils/urlDataStrategy.ts @@ -1,13 +1,9 @@ -import type { - DataStrategyFunction, - DataStrategyFunctionArgs, -} from "../../lib/router"; +import type { DataStrategyFunction } from "../../../lib/router/utils"; import { invariant } from "./utils"; -export default async function urlDataStrategy({ - matches, -}: DataStrategyFunctionArgs): ReturnType { - return Promise.all( +const urlDataStrategy: DataStrategyFunction = async ({ matches }) => { + let results: Record = {}; + await Promise.all( matches.map((match) => match.resolve(async (handler) => { let response = await handler(); @@ -17,11 +13,14 @@ export default async function urlDataStrategy({ contentType === "application/x-www-form-urlencoded", "Invalid Response" ); - return { + results[match.route.id] = { type: "data", result: new URLSearchParams(await response.text()), }; }) ) ); -} + return results; +}; + +export default urlDataStrategy; diff --git a/packages/react-router/__tests__/router/view-transition-test.ts b/packages/react-router/__tests__/router/view-transition-test.ts index 2d140898e7..f67c95540b 100644 --- a/packages/react-router/__tests__/router/view-transition-test.ts +++ b/packages/react-router/__tests__/router/view-transition-test.ts @@ -1,5 +1,6 @@ import { IDLE_NAVIGATION } from "../../lib/router/router"; import { cleanup, setup } from "./utils/data-router-setup"; +import { createFormData } from "./utils/utils"; describe("view transitions", () => { // Detect any failures inside the router navigate code @@ -66,4 +67,109 @@ describe("view transitions", () => { unsubscribe(); t.router.dispose(); }); + + it("preserves pending view transitions through router.revalidate()", async () => { + let t = setup({ + routes: [{ path: "/" }, { id: "a", path: "/a", loader: true }], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let A = await t.navigate("/a", { unstable_viewTransition: true }); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0]).toEqual([ + expect.objectContaining({ + navigation: expect.objectContaining({ state: "loading" }), + }), + expect.objectContaining({ unstable_viewTransitionOpts: undefined }), + ]); + expect(A.loaders.a.stub).toHaveBeenCalledTimes(1); + + // Interrupt the navigation loading state with a revalidation + let B = await t.revalidate(); + expect(spy).toHaveBeenCalledTimes(3); + expect(spy.mock.calls[1]).toEqual([ + expect.objectContaining({ + revalidation: "loading", + }), + expect.objectContaining({ + unstable_viewTransitionOpts: undefined, + }), + ]); + expect(spy.mock.calls[2]).toEqual([ + expect.objectContaining({ + navigation: expect.objectContaining({ state: "loading" }), + }), + expect.objectContaining({ + unstable_viewTransitionOpts: undefined, + }), + ]); + expect(spy).toHaveBeenLastCalledWith( + expect.objectContaining({ + navigation: expect.objectContaining({ state: "loading" }), + }), + expect.objectContaining({ + unstable_viewTransitionOpts: undefined, + }) + ); + expect(B.loaders.a.stub).toHaveBeenCalledTimes(1); + + await A.loaders.a.resolve("A"); + await B.loaders.a.resolve("A*"); + + expect(spy).toHaveBeenCalledTimes(4); + expect(spy.mock.calls[3]).toEqual([ + expect.objectContaining({ + navigation: IDLE_NAVIGATION, + location: expect.objectContaining({ pathname: "/a" }), + loaderData: { + a: "A*", + }, + }), + expect.objectContaining({ + unstable_viewTransitionOpts: { + currentLocation: expect.objectContaining({ pathname: "/" }), + nextLocation: expect.objectContaining({ pathname: "/a" }), + }, + }), + ]); + + unsubscribe(); + t.router.dispose(); + }); + + it("preserves pending view transitions through redirects", async () => { + let t = setup({ + routes: [ + { path: "/" }, + { id: "a", path: "/a", action: true }, + { path: "/b" }, + ], + }); + let spy = jest.fn(); + let unsubscribe = t.router.subscribe(spy); + + let A = await t.navigate("/a", { + formMethod: "post", + formData: createFormData({}), + unstable_viewTransition: true, + }); + + await A.actions.a.redirect("/b"); + expect(spy).toHaveBeenLastCalledWith( + expect.objectContaining({ + navigation: IDLE_NAVIGATION, + location: expect.objectContaining({ pathname: "/b" }), + }), + expect.objectContaining({ + unstable_viewTransitionOpts: { + currentLocation: expect.objectContaining({ pathname: "/" }), + nextLocation: expect.objectContaining({ pathname: "/b" }), + }, + }) + ); + + unsubscribe(); + t.router.dispose(); + }); }); diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 22a44815f8..f741a700a9 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -26,11 +26,11 @@ export type { DataStrategyFunction as unstable_DataStrategyFunction, DataStrategyFunctionArgs as unstable_DataStrategyFunctionArgs, DataStrategyMatch as unstable_DataStrategyMatch, + DataStrategyResult as unstable_DataStrategyResult, DataWithResponseInit as UNSAFE_DataWithResponseInit, ErrorResponse, FormEncType, FormMethod, - HandlerResult as unstable_HandlerResult, HTMLFormMethod, JsonFunction, LazyRouteFunction, diff --git a/packages/react-router/lib/dom-export/hydrated-router.tsx b/packages/react-router/lib/dom-export/hydrated-router.tsx index 991f185c11..a022601686 100644 --- a/packages/react-router/lib/dom-export/hydrated-router.tsx +++ b/packages/react-router/lib/dom-export/hydrated-router.tsx @@ -72,24 +72,6 @@ function createHydratedRouter(): RemixRouter { ); } - // Hard reload if the path we tried to load is not the current path. - // This is usually the result of 2 rapid back/forward clicks from an - // external site into a Remix app, where we initially start the load for - // one URL and while the JS chunks are loading a second forward click moves - // us to a new URL. Avoid comparing search params because of CDNs which - // can be configured to ignore certain params and only pathname is relevant - // towards determining the route matches. - let initialPathname = ssrInfo.context.url; - let hydratedPathname = window.location.pathname; - if (initialPathname !== hydratedPathname && !ssrInfo.context.isSpaMode) { - let errorMsg = - `Initial URL (${initialPathname}) does not match URL at time of hydration ` + - `(${hydratedPathname}), reloading page...`; - console.error(errorMsg); - window.location.reload(); - throw new Error("SSR/Client mismatch - reloading current URL"); - } - // We need to suspend until the initial state snapshot is decoded into // window.__remixContext.state @@ -189,7 +171,8 @@ function createHydratedRouter(): RemixRouter { mapRouteProperties, unstable_dataStrategy: getSingleFetchDataStrategy( ssrInfo.manifest, - ssrInfo.routeModules + ssrInfo.routeModules, + () => router ), unstable_patchRoutesOnNavigation: getPatchRoutesOnNavigationFunction( ssrInfo.manifest, diff --git a/packages/react-router/lib/dom/global.ts b/packages/react-router/lib/dom/global.ts index 27464d650b..da38fe7b17 100644 --- a/packages/react-router/lib/dom/global.ts +++ b/packages/react-router/lib/dom/global.ts @@ -6,7 +6,6 @@ import type { import type { RouteModules } from "./ssr/routeModules"; export type WindowRemixContext = { - url: string; basename?: string; state: HydrationState; criticalCss?: string; @@ -42,7 +41,7 @@ declare global { var __remixManifest: AssetsManifest | undefined; var __remixRouteModules: RouteModules | undefined; var __remixRouter: RemixRouter | undefined; - var __remixRevalidation: number | undefined; + var __remixHdrActive: boolean; var __remixClearCriticalCss: (() => void) | undefined; var $RefreshRuntime$: | { diff --git a/packages/react-router/lib/dom/ssr/components.tsx b/packages/react-router/lib/dom/ssr/components.tsx index 9f83e34a3a..7d1917b825 100644 --- a/packages/react-router/lib/dom/ssr/components.tsx +++ b/packages/react-router/lib/dom/ssr/components.tsx @@ -26,7 +26,7 @@ import type { MetaMatch, MetaMatches, } from "./routeModules"; -import { addRevalidationParam, singleFetchUrl } from "./single-fetch"; +import { singleFetchUrl } from "./single-fetch"; import { DataRouterContext, DataRouterStateContext } from "../../context"; import { useLocation } from "../../hooks"; import { getPartialManifest, isFogOfWarEnabled } from "./fog-of-war"; @@ -325,7 +325,7 @@ function PrefetchPageLinksImpl({ }) { let location = useLocation(); let { manifest, routeModules } = useFrameworkContext(); - let { matches } = useDataRouterStateContext(); + let { loaderData, matches } = useDataRouterStateContext(); let newMatchesForData = React.useMemo( () => @@ -353,6 +353,64 @@ function PrefetchPageLinksImpl({ [page, nextMatches, matches, manifest, location] ); + let dataHrefs = React.useMemo(() => { + if (page === location.pathname + location.search + location.hash) { + // Because we opt-into revalidation, don't compute this for the current page + // since it would always trigger a prefetch of the existing loaders + return []; + } + + // Single-fetch is harder :) + // This parallels the logic in the single fetch data strategy + let routesParams = new Set(); + let foundOptOutRoute = false; + nextMatches.forEach((m) => { + if (!manifest.routes[m.route.id].hasLoader) { + return; + } + + if ( + !newMatchesForData.some((m2) => m2.route.id === m.route.id) && + m.route.id in loaderData && + routeModules[m.route.id]?.shouldRevalidate + ) { + foundOptOutRoute = true; + } else if (manifest.routes[m.route.id].hasClientLoader) { + foundOptOutRoute = true; + } else { + routesParams.add(m.route.id); + } + }); + + if (routesParams.size === 0) { + return []; + } + + let url = singleFetchUrl(page); + // When one or more routes have opted out, we add a _routes param to + // limit the loaders to those that have a server loader and did not + // opt out + if (foundOptOutRoute && routesParams.size > 0) { + url.searchParams.set( + "_routes", + nextMatches + .filter((m) => routesParams.has(m.route.id)) + .map((m) => m.route.id) + .join(",") + ); + } + + return [url.pathname + url.search]; + }, [ + loaderData, + location, + manifest, + newMatchesForData, + nextMatches, + page, + routeModules, + ]); + let moduleHrefs = React.useMemo( () => getModuleLinkHrefs(newMatchesForAssets, manifest), [newMatchesForAssets, manifest] @@ -362,36 +420,11 @@ function PrefetchPageLinksImpl({ // just the manifest like the other links in here. let keyedPrefetchLinks = useKeyedPrefetchLinks(newMatchesForAssets); - let linksToRender: React.ReactNode | React.ReactNode[] | null = null; - if (newMatchesForData.length > 0) { - // Single-fetch with routes that require data - let url = addRevalidationParam( - manifest, - routeModules, - nextMatches.map((m) => m.route), - newMatchesForData.map((m) => m.route), - singleFetchUrl(page) - ); - if (url.searchParams.get("_routes") !== "") { - linksToRender = ( - - ); - } else { - // No single-fetch prefetching if _routes param is empty due to `clientLoader`'s - } - } else { - // No single-fetch prefetching if there are no new matches for data - } - return ( <> - {linksToRender} + {dataHrefs.map((href) => ( + + ))} {moduleHrefs.map((href) => ( ))} diff --git a/packages/react-router/lib/dom/ssr/fallback.tsx b/packages/react-router/lib/dom/ssr/fallback.tsx index e22ccbf41e..148fb6a1b6 100644 --- a/packages/react-router/lib/dom/ssr/fallback.tsx +++ b/packages/react-router/lib/dom/ssr/fallback.tsx @@ -14,8 +14,9 @@ export function RemixRootDefaultHydrateFallback() { __html: ` console.log( "💿 Hey developer 👋. You can provide a way better UX than this " + - "when your app is running \`clientLoader\` functions on hydration. " + - "Check out https://remix.run/route/hydrate-fallback for more information." + "when your app is loading JS modules and/or running \`clientLoader\` " + + "functions. Check out https://remix.run/route/hydrate-fallback " + + "for more information." ); `, }} diff --git a/packages/react-router/lib/dom/ssr/fog-of-war.ts b/packages/react-router/lib/dom/ssr/fog-of-war.ts index 97977d3cb7..eb85a0d19b 100644 --- a/packages/react-router/lib/dom/ssr/fog-of-war.ts +++ b/packages/react-router/lib/dom/ssr/fog-of-war.ts @@ -210,8 +210,8 @@ export async function fetchAndApplyManifestPatches( "/" ); let url = new URL(manifestPath, window.location.origin); + paths.sort().forEach((path) => url.searchParams.append("p", path)); url.searchParams.set("version", manifest.version); - paths.forEach((path) => url.searchParams.append("p", path)); // If the URL is nearing the ~8k limit on GET requests, skip this optimization // step and just let discovery happen on link click. We also wipe out the diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx index d003731dad..d4b3adad4a 100644 --- a/packages/react-router/lib/dom/ssr/single-fetch.tsx +++ b/packages/react-router/lib/dom/ssr/single-fetch.tsx @@ -1,9 +1,11 @@ import * as React from "react"; import { decode } from "turbo-stream"; +import type { Router as RemixRouter } from "../../router/router"; import type { DataStrategyFunction, DataStrategyFunctionArgs, - HandlerResult, + DataStrategyResult, + DataStrategyMatch, } from "../../router/utils"; import { ErrorResponseImpl, @@ -16,7 +18,6 @@ import type { AssetsManifest, EntryContext } from "./entry"; import { escapeHtml } from "./markup"; import type { RouteModules } from "./routeModules"; import invariant from "./invariant"; -import type { DataRouteObject } from "../../context"; export const SingleFetchRedirectSymbol = Symbol("SingleFetchRedirect"); @@ -131,100 +132,244 @@ export function StreamTransfer({ export function getSingleFetchDataStrategy( manifest: AssetsManifest, - routeModules: RouteModules + routeModules: RouteModules, + getRouter: () => RemixRouter ): DataStrategyFunction { - return async ({ request, matches }) => - request.method !== "GET" - ? singleFetchActionStrategy(request, matches) - : singleFetchLoaderStrategy(manifest, routeModules, request, matches); + return async ({ request, matches, fetcherKey }) => { + // Actions are simple and behave the same for navigations and fetchers + if (request.method !== "GET") { + return singleFetchActionStrategy(request, matches); + } + + // Fetcher loads are singular calls to one loader + if (fetcherKey) { + return singleFetchLoaderFetcherStrategy(request, matches); + } + + // Navigational loads are more complex... + return singleFetchLoaderNavigationStrategy( + manifest, + routeModules, + getRouter(), + request, + matches + ); + }; } -// Actions are simple since they're singular calls to the server -function singleFetchActionStrategy( +// Actions are simple since they're singular calls to the server for both +// navigations and fetchers) +async function singleFetchActionStrategy( request: Request, matches: DataStrategyFunctionArgs["matches"] ) { - return Promise.all( - matches.map(async (m) => { - let actionStatus: number | undefined; - let result = await m.resolve(async (handler): Promise => { - let result = await handler(async () => { - let url = singleFetchUrl(request.url); - let init = await createRequestInit(request); - let { data, status } = await fetchAndDecode(url, init); - actionStatus = status; - return unwrapSingleFetchResult(data as SingleFetchResult, m.route.id); - }); - return { type: "data", result }; - }); + let actionMatch = matches.find((m) => m.shouldLoad); + invariant(actionMatch, "No action match found"); + let actionStatus: number | undefined = undefined; + let result = await actionMatch.resolve(async (handler) => { + let result = await handler(async () => { + let url = singleFetchUrl(request.url); + let init = await createRequestInit(request); + let { data, status } = await fetchAndDecode(url, init); + actionStatus = status; + return unwrapSingleFetchResult( + data as SingleFetchResult, + actionMatch!.route.id + ); + }); + return result; + }); - if (isResponse(result.result) || isRouteErrorResponse(result.result)) { - return result; - } + if (isResponse(result.result) || isRouteErrorResponse(result.result)) { + return { [actionMatch.route.id]: result }; + } - // For non-responses, proxy along the statusCode via unstable_data() - // (most notably for skipping action error revalidation) - return { - type: result.type, - result: data(result.result, actionStatus), - }; - }) - ); + // For non-responses, proxy along the statusCode via unstable_data() + // (most notably for skipping action error revalidation) + return { + [actionMatch.route.id]: { + type: result.type, + result: data(result.result, actionStatus), + }, + }; } // Loaders are trickier since we only want to hit the server once, so we // create a singular promise for all server-loader routes to latch onto. -function singleFetchLoaderStrategy( +async function singleFetchLoaderNavigationStrategy( manifest: AssetsManifest, routeModules: RouteModules, + router: RemixRouter, request: Request, matches: DataStrategyFunctionArgs["matches"] ) { - let singleFetchPromise: Promise | undefined; - return Promise.all( - matches.map(async (m) => - m.resolve(async (handler): Promise => { - let result: unknown; - let url = stripIndexParam(singleFetchUrl(request.url)); - let init = await createRequestInit(request); - - // When a route has a client loader, it calls it's singular server loader + // Track which routes need a server load - in case we need to tack on a + // `_routes` param + let routesParams = new Set(); + + // We only add `_routes` when one or more routes opts out of a load via + // `shouldRevalidate` or `clientLoader` + let foundOptOutRoute = false; + + // Deferreds for each route so we can be sure they've all loaded via + // `match.resolve()`, and a singular promise that can tell us all routes + // have been resolved + let routeDfds = matches.map(() => createDeferred()); + let routesLoadedPromise = Promise.all(routeDfds.map((d) => d.promise)); + + // Deferred that we'll use for the call to the server that each match can + // await and parse out it's specific result + let singleFetchDfd = createDeferred(); + + // Base URL and RequestInit for calls to the server + let url = stripIndexParam(singleFetchUrl(request.url)); + let init = await createRequestInit(request); + + // We'll build up this results object as we loop through matches + let results: Record = {}; + + let resolvePromise = Promise.all( + matches.map(async (m, i) => + m.resolve(async (handler) => { + routeDfds[i].resolve(); + + if (!m.shouldLoad) { + // If we're not yet initialized and this is the initial load, respect + // `shouldLoad` because we're only dealing with `clientLoader.hydrate` + // routes which will fall into the `clientLoader` section below. + if (!router.state.initialized) { + return; + } + + // Otherwise, we opt out if we currently have data, a `loader`, and a + // `shouldRevalidate` function. This implies that the user opted out + // via `shouldRevalidate` + if ( + m.route.id in router.state.loaderData && + manifest.routes[m.route.id].hasLoader && + routeModules[m.route.id]?.shouldRevalidate + ) { + foundOptOutRoute = true; + return; + } + } + + // When a route has a client loader, it opts out of the singular call and + // calls it's server loader via `serverLoader()` using a `?_routes` param if (manifest.routes[m.route.id].hasClientLoader) { - result = await handler(async () => { - url.searchParams.set("_routes", m.route.id); - let { data } = await fetchAndDecode(url, init); - return unwrapSingleFetchResults( - data as SingleFetchResults, + if (manifest.routes[m.route.id].hasLoader) { + foundOptOutRoute = true; + } + try { + let result = await fetchSingleLoader( + handler, + url, + init, m.route.id ); - }); - } else { - result = await handler(async () => { - // Otherwise we let multiple routes hook onto the same promise - if (!singleFetchPromise) { - url = addRevalidationParam( - manifest, - routeModules, - matches.map((m) => m.route), - matches.filter((m) => m.shouldLoad).map((m) => m.route), - url - ); - singleFetchPromise = fetchAndDecode(url, init).then( - ({ data }) => data as SingleFetchResults - ); - } - let results = await singleFetchPromise; - return unwrapSingleFetchResults(results, m.route.id); - }); + results[m.route.id] = { type: "data", result }; + } catch (e) { + results[m.route.id] = { type: "error", result: e }; + } + return; } - return { - type: "data", - result, - }; + // Load this route on the server if it has a loader + if (manifest.routes[m.route.id].hasLoader) { + routesParams.add(m.route.id); + } + + // Lump this match in with the others on a singular promise + try { + let result = await handler(async () => { + let data = await singleFetchDfd.promise; + return unwrapSingleFetchResults(data, m.route.id); + }); + results[m.route.id] = { + type: "data", + result, + }; + } catch (e) { + results[m.route.id] = { + type: "error", + result: e, + }; + } }) ) ); + + // Wait for all routes to resolve above before we make the HTTP call + await routesLoadedPromise; + + // We can skip the server call: + // - On initial hydration - only clientLoaders can pass through via `clientLoader.hydrate` + // - If there are no routes to fetch from the server + // + // One exception - if we are performing an HDR revalidation we have to call + // the server in case a new loader has shown up that the manifest doesn't yet + // know about + if ( + (!router.state.initialized || routesParams.size === 0) && + !window.__remixHdrActive + ) { + singleFetchDfd.resolve({}); + } else { + try { + // When one or more routes have opted out, we add a _routes param to + // limit the loaders to those that have a server loader and did not + // opt out + if (foundOptOutRoute && routesParams.size > 0) { + url.searchParams.set( + "_routes", + matches + .filter((m) => routesParams.has(m.route.id)) + .map((m) => m.route.id) + .join(",") + ); + } + + let data = await fetchAndDecode(url, init); + singleFetchDfd.resolve(data.data as SingleFetchResults); + } catch (e) { + singleFetchDfd.reject(e as Error); + } + } + + await resolvePromise; + + return results; +} + +// Fetcher loader calls are much simpler than navigational loader calls +async function singleFetchLoaderFetcherStrategy( + request: Request, + matches: DataStrategyFunctionArgs["matches"] +) { + let fetcherMatch = matches.find((m) => m.shouldLoad); + invariant(fetcherMatch, "No fetcher match found"); + let result = await fetcherMatch.resolve(async (handler) => { + let url = stripIndexParam(singleFetchUrl(request.url)); + let init = await createRequestInit(request); + return fetchSingleLoader(handler, url, init, fetcherMatch!.route.id); + }); + return { [fetcherMatch.route.id]: result }; +} + +function fetchSingleLoader( + handler: Parameters< + NonNullable[0]> + >[0], + url: URL, + init: RequestInit, + routeId: string +) { + return handler(async () => { + let singleLoaderUrl = new URL(url); + singleLoaderUrl.searchParams.set("_routes", routeId); + let { data } = await fetchAndDecode(singleLoaderUrl, init); + return unwrapSingleFetchResults(data as SingleFetchResults, routeId); + }); } function stripIndexParam(url: URL) { @@ -243,56 +388,6 @@ function stripIndexParam(url: URL) { return url; } -// Determine which routes we want to load so we can add a `?_routes` search param -// for fine-grained revalidation if necessary. There's some nuance to this decision: -// -// - The presence of `shouldRevalidate` and `clientLoader` functions are the only -// way to trigger fine-grained single fetch loader calls. without either of -// these on the route matches we just always ask for the full `.data` request. -// - If any routes have a `shouldRevalidate` or `clientLoader` then we do a -// comparison of the routes we matched and the routes we're aiming to load -// - If they don't match up, then we add the `_routes` param or fine-grained -// loading -// - This is used by the single fetch implementation above and by the -// `` component so we can prefetch routes using the -// same logic -export function addRevalidationParam( - manifest: AssetsManifest, - routeModules: RouteModules, - matchedRoutes: DataRouteObject[], - loadRoutes: DataRouteObject[], - url: URL -) { - let genRouteIds = (arr: string[]) => - arr.filter((id) => manifest.routes[id].hasLoader).join(","); - - // Look at the `routeModules` for `shouldRevalidate` here instead of the manifest - // since HDR adds a wrapper for `shouldRevalidate` even if the route didn't have one - // initially. - // TODO: We probably can get rid of that wrapper once we're strictly on on - // single-fetch in v3 and just leverage a needsRevalidation data structure here - // to determine what to fetch - let needsParam = matchedRoutes.some( - (r) => - routeModules[r.id]?.shouldRevalidate || - manifest.routes[r.id]?.hasClientLoader - ); - if (!needsParam) { - return url; - } - - let matchedIds = genRouteIds(matchedRoutes.map((r) => r.id)); - let loadIds = genRouteIds( - loadRoutes - .filter((r) => !manifest.routes[r.id]?.hasClientLoader) - .map((r) => r.id) - ); - if (matchedIds !== loadIds) { - url.searchParams.set("_routes", loadIds); - } - return url; -} - export function singleFetchUrl(reqUrl: URL | string) { let url = typeof reqUrl === "string" @@ -414,3 +509,29 @@ function unwrapSingleFetchResult(result: SingleFetchResult, routeId: string) { throw new Error(`No response found for routeId "${routeId}"`); } } + +function createDeferred() { + let resolve: (val?: any) => Promise; + let reject: (error?: Error) => Promise; + let promise = new Promise((res, rej) => { + resolve = async (val: T) => { + res(val); + try { + await promise; + } catch (e) {} + }; + reject = async (error?: Error) => { + rej(error); + try { + await promise; + } catch (e) {} + }; + }); + return { + promise, + //@ts-ignore + resolve, + //@ts-ignore + reject, + }; +} diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 4b983f6265..bb8eb711a7 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -19,7 +19,7 @@ import type { FormEncType, FormMethod, HTMLFormMethod, - HandlerResult, + DataStrategyResult, ImmutableRouteKey, MapRoutePropertiesFunction, MutationFormMethod, @@ -1022,7 +1022,7 @@ export function createRouter(init: RouterInit): Router { // Flag to ignore the next history update, so we can revert the URL change on // a POP navigation that was blocked by the user without touching router state - let ignoreNextHistoryUpdate = false; + let unblockBlockerHistoryUpdate: (() => void) | undefined = undefined; let pendingRevalidationDfd: ReturnType> | null = null; @@ -1037,8 +1037,9 @@ export function createRouter(init: RouterInit): Router { ({ action: historyAction, location, delta }) => { // Ignore this event if it was just us resetting the URL from a // blocked POP navigation - if (ignoreNextHistoryUpdate) { - ignoreNextHistoryUpdate = false; + if (unblockBlockerHistoryUpdate) { + unblockBlockerHistoryUpdate(); + unblockBlockerHistoryUpdate = undefined; return; } @@ -1060,7 +1061,9 @@ export function createRouter(init: RouterInit): Router { if (blockerKey && delta != null) { // Restore the URL to match the current UI, but don't update router state - ignoreNextHistoryUpdate = true; + let nextHistoryUpdatePromise = new Promise((resolve) => { + unblockBlockerHistoryUpdate = resolve; + }); init.history.go(delta * -1); // Put the blocker into a blocked state @@ -1074,8 +1077,10 @@ export function createRouter(init: RouterInit): Router { reset: undefined, location, }); - // Re-do the same POP navigation we just blocked - init.history.go(delta); + // Re-do the same POP navigation we just blocked, after the url + // restoration is also complete. See: + // https://github.com/remix-run/react-router/issues/11613 + nextHistoryUpdatePromise.then(() => init.history.go(delta)); }, reset() { let blockers = new Map(state.blockers); @@ -1483,7 +1488,11 @@ export function createRouter(init: RouterInit): Router { startNavigation( pendingAction || state.historyAction, state.navigation.location, - { overrideNavigation: state.navigation } + { + overrideNavigation: state.navigation, + // Proxy through any rending view transition + enableViewTransition: pendingViewTransitionEnabled === true, + } ); return promise; } @@ -1754,11 +1763,13 @@ export function createRouter(init: RouterInit): Router { } else { let results = await callDataStrategy( "action", + state, request, [actionMatch], - matches + matches, + null ); - result = results[0]; + result = results[actionMatch.route.id]; if (request.signal.aborted) { return { shortCircuited: true }; @@ -1780,7 +1791,7 @@ export function createRouter(init: RouterInit): Router { ); replace = location === state.location.pathname + state.location.search; } - await startRedirectNavigation(request, result, { + await startRedirectNavigation(request, result, true, { submission, replace, }); @@ -1984,7 +1995,7 @@ export function createRouter(init: RouterInit): Router { let { loaderResults, fetcherResults } = await callLoadersAndMaybeResolveData( - state.matches, + state, matches, matchesToLoad, revalidatingFetchers, @@ -2007,17 +2018,21 @@ export function createRouter(init: RouterInit): Router { revalidatingFetchers.forEach((rf) => fetchControllers.delete(rf.key)); // If any loaders returned a redirect Response, start a new REPLACE navigation - let redirect = findRedirect([...loaderResults, ...fetcherResults]); + let redirect = findRedirect(loaderResults); if (redirect) { - if (redirect.idx >= matchesToLoad.length) { - // If this redirect came from a fetcher make sure we mark it in - // fetchRedirectIds so it doesn't get revalidated on the next set of - // loader executions - let fetcherKey = - revalidatingFetchers[redirect.idx - matchesToLoad.length].key; - fetchRedirectIds.add(fetcherKey); - } - await startRedirectNavigation(request, redirect.result, { + await startRedirectNavigation(request, redirect.result, true, { + replace, + }); + return { shortCircuited: true }; + } + + redirect = findRedirect(fetcherResults); + if (redirect) { + // If this redirect came from a fetcher make sure we mark it in + // fetchRedirectIds so it doesn't get revalidated on the next set of + // loader executions + fetchRedirectIds.add(redirect.key); + await startRedirectNavigation(request, redirect.result, true, { replace, }); return { shortCircuited: true }; @@ -2252,11 +2267,13 @@ export function createRouter(init: RouterInit): Router { let originatingLoadId = incrementingLoadId; let actionResults = await callDataStrategy( "action", + state, fetchRequest, [match], - requestMatches + requestMatches, + key ); - let actionResult = actionResults[0]; + let actionResult = actionResults[match.route.id]; if (fetchRequest.signal.aborted) { // We can delete this so long as we weren't aborted by our own fetcher @@ -2288,7 +2305,7 @@ export function createRouter(init: RouterInit): Router { } else { fetchRedirectIds.add(key); updateFetcherState(key, getLoadingFetcher(submission)); - return startRedirectNavigation(fetchRequest, actionResult, { + return startRedirectNavigation(fetchRequest, actionResult, false, { fetcherSubmission: submission, }); } @@ -2373,7 +2390,7 @@ export function createRouter(init: RouterInit): Router { let { loaderResults, fetcherResults } = await callLoadersAndMaybeResolveData( - state.matches, + state, matches, matchesToLoad, revalidatingFetchers, @@ -2393,23 +2410,32 @@ export function createRouter(init: RouterInit): Router { fetchControllers.delete(key); revalidatingFetchers.forEach((r) => fetchControllers.delete(r.key)); - let redirect = findRedirect([...loaderResults, ...fetcherResults]); + let redirect = findRedirect(loaderResults); if (redirect) { - if (redirect.idx >= matchesToLoad.length) { - // If this redirect came from a fetcher make sure we mark it in - // fetchRedirectIds so it doesn't get revalidated on the next set of - // loader executions - let fetcherKey = - revalidatingFetchers[redirect.idx - matchesToLoad.length].key; - fetchRedirectIds.add(fetcherKey); - } - return startRedirectNavigation(revalidationRequest, redirect.result); + return startRedirectNavigation( + revalidationRequest, + redirect.result, + false + ); + } + + redirect = findRedirect(fetcherResults); + if (redirect) { + // If this redirect came from a fetcher make sure we mark it in + // fetchRedirectIds so it doesn't get revalidated on the next set of + // loader executions + fetchRedirectIds.add(redirect.key); + return startRedirectNavigation( + revalidationRequest, + redirect.result, + false + ); } // Process and commit output from loaders let { loaderData, errors } = processLoaderData( state, - state.matches, + matches, matchesToLoad, loaderResults, undefined, @@ -2521,11 +2547,13 @@ export function createRouter(init: RouterInit): Router { let originatingLoadId = incrementingLoadId; let results = await callDataStrategy( "loader", + state, fetchRequest, [match], - matches + matches, + key ); - let result = results[0]; + let result = results[match.route.id]; // We can delete this so long as we weren't aborted by our our own fetcher // re-load which would have put _new_ controller is in fetchControllers @@ -2553,7 +2581,7 @@ export function createRouter(init: RouterInit): Router { return; } else { fetchRedirectIds.add(key); - await startRedirectNavigation(fetchRequest, result); + await startRedirectNavigation(fetchRequest, result, false); return; } } @@ -2590,6 +2618,7 @@ export function createRouter(init: RouterInit): Router { async function startRedirectNavigation( request: Request, redirect: RedirectResult, + isNavigation: boolean, { submission, fetcherSubmission, @@ -2676,8 +2705,11 @@ export function createRouter(init: RouterInit): Router { ...activeSubmission, formAction: location, }, - // Preserve this flag across redirects + // Preserve these flags across redirects preventScrollReset: pendingPreventScrollReset, + enableViewTransition: isNavigation + ? pendingViewTransitionEnabled + : undefined, }); } else { // If we have a navigation submission, we will preserve it through the @@ -2690,8 +2722,11 @@ export function createRouter(init: RouterInit): Router { overrideNavigation, // Send fetcher submissions through for shouldRevalidate fetcherSubmission, - // Preserve this flag across redirects + // Preserve these flags across redirects preventScrollReset: pendingPreventScrollReset, + enableViewTransition: isNavigation + ? pendingViewTransitionEnabled + : undefined, }); } } @@ -2700,84 +2735,112 @@ export function createRouter(init: RouterInit): Router { // pass around the manifest, mapRouteProperties, etc. async function callDataStrategy( type: "loader" | "action", + state: RouterState, request: Request, matchesToLoad: AgnosticDataRouteMatch[], - matches: AgnosticDataRouteMatch[] - ): Promise { + matches: AgnosticDataRouteMatch[], + fetcherKey: string | null + ): Promise> { + let results: Record; + let dataResults: Record = {}; try { - let results = await callDataStrategyImpl( + results = await callDataStrategyImpl( dataStrategyImpl, type, + state, request, matchesToLoad, matches, + fetcherKey, manifest, mapRouteProperties ); - - return await Promise.all( - results.map((result, i) => { - if (isRedirectHandlerResult(result)) { - let response = result.result as Response; - return { - type: ResultType.redirect, - response: normalizeRelativeRoutingRedirectResponse( - response, - request, - matchesToLoad[i].route.id, - matches, - basename - ), - }; - } - - return convertHandlerResultToDataResult(result); - }) - ); } catch (e) { // If the outer dataStrategy method throws, just return the error for all // matches - and it'll naturally bubble to the root - return matchesToLoad.map(() => ({ - type: ResultType.error, - error: e, - })); + matchesToLoad.forEach((m) => { + dataResults[m.route.id] = { + type: ResultType.error, + error: e, + }; + }); + return dataResults; } + + for (let [routeId, result] of Object.entries(results)) { + if (isRedirectDataStrategyResultResult(result)) { + let response = result.result as Response; + dataResults[routeId] = { + type: ResultType.redirect, + response: normalizeRelativeRoutingRedirectResponse( + response, + request, + routeId, + matches, + basename + ), + }; + } else { + dataResults[routeId] = await convertDataStrategyResultToDataResult( + result + ); + } + } + + return dataResults; } async function callLoadersAndMaybeResolveData( - currentMatches: AgnosticDataRouteMatch[], + state: RouterState, matches: AgnosticDataRouteMatch[], matchesToLoad: AgnosticDataRouteMatch[], fetchersToLoad: RevalidatingFetcher[], request: Request ) { - let [loaderResults, ...fetcherResults] = await Promise.all([ - matchesToLoad.length - ? callDataStrategy("loader", request, matchesToLoad, matches) - : [], - ...fetchersToLoad.map((f) => { + let currentMatches = state.matches; + + // Kick off loaders and fetchers in parallel + let loaderResultsPromise = callDataStrategy( + "loader", + state, + request, + matchesToLoad, + matches, + null + ); + + let fetcherResultsPromise = Promise.all( + fetchersToLoad.map(async (f) => { if (f.matches && f.match && f.controller) { - let fetcherRequest = createClientSideRequest( - init.history, - f.path, - f.controller.signal - ); - return callDataStrategy( + let results = await callDataStrategy( "loader", - fetcherRequest, + state, + createClientSideRequest(init.history, f.path, f.controller.signal), [f.match], - f.matches - ).then((r) => r[0]); + f.matches, + f.key + ); + let result = results[f.match.route.id]; + // Fetcher results are keyed by fetcher key from here on out, not routeId + return { [f.key]: result }; } else { - return Promise.resolve({ - type: ResultType.error, - error: getInternalRouterError(404, { - pathname: f.path, - }), + return Promise.resolve({ + [f.key]: { + type: ResultType.error, + error: getInternalRouterError(404, { + pathname: f.path, + }), + } as ErrorResult, }); } - }), - ]); + }) + ); + + let loaderResults = await loaderResultsPromise; + let fetcherResults = (await fetcherResultsPromise).reduce( + (acc, r) => Object.assign(acc, r), + {} + ); return { loaderResults, @@ -3559,9 +3622,9 @@ export function createStaticHandler( }; } catch (e) { // If the user threw/returned a Response in callLoaderOrAction for a - // `queryRoute` call, we throw the `HandlerResult` to bail out early + // `queryRoute` call, we throw the `DataStrategyResult` to bail out early // and then return or throw the raw Response here accordingly - if (isHandlerResult(e) && isResponse(e.result)) { + if (isDataStrategyResult(e) && isResponse(e.result)) { if (e.type === ResultType.error) { throw e.result; } @@ -3610,7 +3673,7 @@ export function createStaticHandler( requestContext, unstable_dataStrategy ); - result = results[0]; + result = results[actionMatch.route.id]; if (request.signal.aborted) { throwStaticHandlerAbortedError(request, isRouteRequest); @@ -3787,7 +3850,6 @@ export function createStaticHandler( // Process and commit output from loaders let context = processRouteLoaderData( matches, - matchesToLoad, results, pendingActionResult, skipLoaderErrorBubbling @@ -3819,27 +3881,34 @@ export function createStaticHandler( isRouteRequest: boolean, requestContext: unknown, unstable_dataStrategy: DataStrategyFunction | null - ): Promise { + ): Promise> { let results = await callDataStrategyImpl( unstable_dataStrategy || defaultDataStrategy, type, + null, request, matchesToLoad, matches, + null, manifest, mapRouteProperties, requestContext ); - return await Promise.all( - results.map((result, i) => { - if (isRedirectHandlerResult(result)) { + let dataResults: Record = {}; + await Promise.all( + matches.map(async (match) => { + if (!(match.route.id in results)) { + return; + } + let result = results[match.route.id]; + if (isRedirectDataStrategyResultResult(result)) { let response = result.result as Response; // Throw redirects and let the server handle them with an HTTP redirect throw normalizeRelativeRoutingRedirectResponse( response, request, - matchesToLoad[i].route.id, + match.route.id, matches, basename ); @@ -3850,9 +3919,11 @@ export function createStaticHandler( throw result; } - return convertHandlerResultToDataResult(result); + dataResults[match.route.id] = + await convertDataStrategyResultToDataResult(result); }) ); + return dataResults; } return { @@ -4533,77 +4604,91 @@ async function loadLazyRouteModule( } // Default implementation of `dataStrategy` which fetches all loaders in parallel -function defaultDataStrategy( - opts: DataStrategyFunctionArgs -): ReturnType { - return Promise.all(opts.matches.map((m) => m.resolve())); +async function defaultDataStrategy({ + matches, +}: DataStrategyFunctionArgs): ReturnType { + let matchesToLoad = matches.filter((m) => m.shouldLoad); + let results = await Promise.all(matchesToLoad.map((m) => m.resolve())); + return results.reduce( + (acc, result, i) => + Object.assign(acc, { [matchesToLoad[i].route.id]: result }), + {} + ); } async function callDataStrategyImpl( dataStrategyImpl: DataStrategyFunction, type: "loader" | "action", + state: RouterState | null, request: Request, matchesToLoad: AgnosticDataRouteMatch[], matches: AgnosticDataRouteMatch[], + fetcherKey: string | null, manifest: RouteManifest, mapRouteProperties: MapRoutePropertiesFunction, requestContext?: unknown -): Promise { - let routeIdsToLoad = matchesToLoad.reduce( - (acc, m) => acc.add(m.route.id), - new Set() +): Promise> { + let loadRouteDefinitionsPromises = matches.map((m) => + m.route.lazy + ? loadLazyRouteModule(m.route, mapRouteProperties, manifest) + : undefined ); - let loadedMatches = new Set(); + + let dsMatches = matches.map((match, i) => { + let loadRoutePromise = loadRouteDefinitionsPromises[i]; + let shouldLoad = matchesToLoad.some((m) => m.route.id === match.route.id); + // `resolve` encapsulates route.lazy(), executing the loader/action, + // and mapping return values/thrown errors to a `DataStrategyResult`. Users + // can pass a callback to take fine-grained control over the execution + // of the loader/action + let resolve: DataStrategyMatch["resolve"] = async (handlerOverride) => { + if ( + handlerOverride && + request.method === "GET" && + (match.route.lazy || match.route.loader) + ) { + shouldLoad = true; + } + return shouldLoad + ? callLoaderOrAction( + type, + request, + match, + loadRoutePromise, + handlerOverride, + requestContext + ) + : Promise.resolve({ type: ResultType.data, result: undefined }); + }; + + return { + ...match, + shouldLoad, + resolve, + }; + }); // Send all matches here to allow for a middleware-type implementation. // handler will be a no-op for unneeded routes and we filter those results // back out below. let results = await dataStrategyImpl({ - matches: matches.map((match) => { - let shouldLoad = routeIdsToLoad.has(match.route.id); - // `resolve` encapsulates the route.lazy, executing the - // loader/action, and mapping return values/thrown errors to a - // HandlerResult. Users can pass a callback to take fine-grained control - // over the execution of the loader/action - let resolve: DataStrategyMatch["resolve"] = (handlerOverride) => { - loadedMatches.add(match.route.id); - return shouldLoad - ? callLoaderOrAction( - type, - request, - match, - manifest, - mapRouteProperties, - handlerOverride, - requestContext - ) - : Promise.resolve({ type: ResultType.data, result: undefined }); - }; - - return { - ...match, - shouldLoad, - resolve, - }; - }), + matches: dsMatches, request, params: matches[0].params, + fetcherKey, context: requestContext, }); - // Throw if any loadRoute implementations not called since they are what - // ensures a route is fully loaded - matches.forEach((m) => - invariant( - loadedMatches.has(m.route.id), - `\`match.resolve()\` was not called for route id "${m.route.id}". ` + - "You must call `match.resolve()` on every match passed to " + - "`dataStrategy` to ensure all routes are properly loaded." - ) - ); + // Wait for all routes to load here but 'swallow the error since we want + // it to bubble up from the `await loadRoutePromise` in `callLoaderOrAction` - + // called from `match.resolve()` + try { + await Promise.all(loadRouteDefinitionsPromises); + } catch (e) { + // No-op + } - // Filter out any middleware-only matches for which we didn't need to run handlers - return results.filter((_, i) => routeIdsToLoad.has(matches[i].route.id)); + return results; } // Default logic for calling a loader/action is the user has no specified a dataStrategy @@ -4611,22 +4696,21 @@ async function callLoaderOrAction( type: "loader" | "action", request: Request, match: AgnosticDataRouteMatch, - manifest: RouteManifest, - mapRouteProperties: MapRoutePropertiesFunction, + loadRoutePromise: Promise | undefined, handlerOverride: Parameters[0], staticContext?: unknown -): Promise { - let result: HandlerResult; +): Promise { + let result: DataStrategyResult; let onReject: (() => void) | undefined; let runHandler = ( handler: AgnosticRouteObject["loader"] | AgnosticRouteObject["action"] - ): Promise => { + ): Promise => { // Setup a promise we can race against so that abort signals short circuit let reject: () => void; - // This will never resolve so safe to type it as Promise to + // This will never resolve so safe to type it as Promise to // satisfy the function return value - let abortPromise = new Promise((_, r) => (reject = r)); + let abortPromise = new Promise((_, r) => (reject = r)); onReject = () => reject(); request.signal.addEventListener("abort", onReject); @@ -4649,19 +4733,16 @@ async function callLoaderOrAction( ); }; - let handlerPromise: Promise; - if (handlerOverride) { - handlerPromise = handlerOverride((ctx: unknown) => actualHandler(ctx)); - } else { - handlerPromise = (async () => { - try { - let val = await actualHandler(); - return { type: "data", result: val }; - } catch (e) { - return { type: "error", result: e }; - } - })(); - } + let handlerPromise: Promise = (async () => { + try { + let val = await (handlerOverride + ? handlerOverride((ctx: unknown) => actualHandler(ctx)) + : actualHandler()); + return { type: "data", result: val }; + } catch (e) { + return { type: "error", result: e }; + } + })(); return Promise.race([handlerPromise, abortPromise]); }; @@ -4669,7 +4750,8 @@ async function callLoaderOrAction( try { let handler = match.route[type]; - if (match.route.lazy) { + // If we have a route.lazy promise, await that first + if (loadRoutePromise) { if (handler) { // Run statically defined handler in parallel with lazy() let handlerError; @@ -4680,7 +4762,7 @@ async function callLoaderOrAction( runHandler(handler).catch((e) => { handlerError = e; }), - loadLazyRouteModule(match.route, mapRouteProperties, manifest), + loadRoutePromise, ]); if (handlerError !== undefined) { throw handlerError; @@ -4688,7 +4770,7 @@ async function callLoaderOrAction( result = value!; } else { // Load lazy route module, then run any returned handler - await loadLazyRouteModule(match.route, mapRouteProperties, manifest); + await loadRoutePromise; handler = match.route[type]; if (handler) { @@ -4721,7 +4803,7 @@ async function callLoaderOrAction( } } catch (e) { // We should already be catching and converting normal handler executions to - // HandlerResults and returning them, so anything that throws here is an + // DataStrategyResults and returning them, so anything that throws here is an // unexpected error we still need to wrap return { type: ResultType.error, result: e }; } finally { @@ -4733,10 +4815,10 @@ async function callLoaderOrAction( return result; } -async function convertHandlerResultToDataResult( - handlerResult: HandlerResult +async function convertDataStrategyResultToDataResult( + dataStrategyResult: DataStrategyResult ): Promise { - let { result, type } = handlerResult; + let { result, type } = dataStrategyResult; if (isResponse(result)) { let data: any; @@ -4927,8 +5009,7 @@ function convertSearchParamsToFormData( function processRouteLoaderData( matches: AgnosticDataRouteMatch[], - matchesToLoad: AgnosticDataRouteMatch[], - results: DataResult[], + results: Record, pendingActionResult: PendingActionResult | undefined, skipLoaderErrorBubbling: boolean ): { @@ -4949,8 +5030,12 @@ function processRouteLoaderData( : undefined; // Process loader results into state.loaderData/state.errors - results.forEach((result, index) => { - let id = matchesToLoad[index].route.id; + matches.forEach((match) => { + if (!(match.route.id in results)) { + return; + } + let id = match.route.id; + let result = results[id]; invariant( !isRedirectResult(result), "Cannot handle redirect results in processLoaderData" @@ -5026,35 +5111,31 @@ function processLoaderData( state: RouterState, matches: AgnosticDataRouteMatch[], matchesToLoad: AgnosticDataRouteMatch[], - results: DataResult[], + results: Record, pendingActionResult: PendingActionResult | undefined, revalidatingFetchers: RevalidatingFetcher[], - fetcherResults: DataResult[] + fetcherResults: Record ): { loaderData: RouterState["loaderData"]; errors?: RouterState["errors"]; } { let { loaderData, errors } = processRouteLoaderData( matches, - matchesToLoad, results, pendingActionResult, false // This method is only called client side so we always want to bubble ); // Process results from our revalidating fetchers - for (let index = 0; index < revalidatingFetchers.length; index++) { - let { key, match, controller } = revalidatingFetchers[index]; - invariant( - fetcherResults !== undefined && fetcherResults[index] !== undefined, - "Did not find corresponding fetcher result" - ); - let result = fetcherResults[index]; + revalidatingFetchers.forEach((rf) => { + let { key, match, controller } = rf; + let result = fetcherResults[key]; + invariant(result, "Did not find corresponding fetcher result"); // Process fetcher non-redirect errors if (controller && controller.signal.aborted) { // Nothing to do for aborted fetchers - continue; + return; } else if (isErrorResult(result)) { let boundaryMatch = findNearestBoundary(state.matches, match?.route.id); if (!(errors && errors[boundaryMatch.route.id])) { @@ -5072,7 +5153,7 @@ function processLoaderData( let doneFetcher = getDoneFetcher(result.data); state.fetchers.set(key, doneFetcher); } - } + }); return { loaderData, errors }; } @@ -5228,12 +5309,13 @@ function getInternalRouterError( // Find any returned redirect errors, starting from the lowest match function findRedirect( - results: DataResult[] -): { result: RedirectResult; idx: number } | undefined { - for (let i = results.length - 1; i >= 0; i--) { - let result = results[i]; + results: Record +): { key: string; result: RedirectResult } | undefined { + let entries = Object.entries(results); + for (let i = entries.length - 1; i >= 0; i--) { + let [key, result] = entries[i]; if (isRedirectResult(result)) { - return { result, idx: i }; + return { key, result }; } } } @@ -5268,7 +5350,7 @@ function isPromise(val: unknown): val is Promise { return typeof val === "object" && val != null && "then" in val; } -function isHandlerResult(result: unknown): result is HandlerResult { +function isDataStrategyResult(result: unknown): result is DataStrategyResult { return ( result != null && typeof result === "object" && @@ -5278,7 +5360,7 @@ function isHandlerResult(result: unknown): result is HandlerResult { ); } -function isRedirectHandlerResult(result: HandlerResult) { +function isRedirectDataStrategyResultResult(result: DataStrategyResult) { return ( isResponse(result.result) && redirectStatusCodes.has(result.result.status) ); diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index f08019ef87..06a4eee44e 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -48,14 +48,6 @@ export interface ErrorResult { */ export type DataResult = SuccessResult | RedirectResult | ErrorResult; -/** - * Result from a loader or action called via dataStrategy - */ -export interface HandlerResult { - type: "data" | "error"; - result: unknown; // data, Error, Response, DataWithResponseInit -} - export type LowerCaseFormMethod = "get" | "post" | "put" | "patch" | "delete"; export type UpperCaseFormMethod = Uppercase; @@ -209,17 +201,26 @@ export interface DataStrategyMatch resolve: ( handlerOverride?: ( handler: (ctx?: unknown) => DataFunctionReturnValue - ) => Promise - ) => Promise; + ) => DataFunctionReturnValue + ) => Promise; } export interface DataStrategyFunctionArgs extends DataFunctionArgs { matches: DataStrategyMatch[]; + fetcherKey: string | null; +} + +/** + * Result from a loader or action called via dataStrategy + */ +export interface DataStrategyResult { + type: "data" | "error"; + result: unknown; // data, Error, Response, DeferredData, DataWithResponseInit } export interface DataStrategyFunction { - (args: DataStrategyFunctionArgs): Promise; + (args: DataStrategyFunctionArgs): Promise>; } export interface AgnosticPatchRoutesOnNavigationFunction< diff --git a/packages/react-router/lib/server-runtime/data.ts b/packages/react-router/lib/server-runtime/data.ts index d07728572a..035c477cf8 100644 --- a/packages/react-router/lib/server-runtime/data.ts +++ b/packages/react-router/lib/server-runtime/data.ts @@ -34,7 +34,7 @@ export async function callRouteAction({ routeId: string; }) { let result = await action({ - request: stripDataParam(stripIndexParam(request)), + request: stripRoutesParam(stripIndexParam(request)), context: loadContext, params, }); @@ -63,7 +63,7 @@ export async function callRouteLoader({ routeId: string; }) { let result = await loader({ - request: stripDataParam(stripIndexParam(request)), + request: stripRoutesParam(stripIndexParam(request)), context: loadContext, params, }); @@ -111,9 +111,9 @@ function stripIndexParam(request: Request) { return new Request(url.href, init); } -function stripDataParam(request: Request) { +function stripRoutesParam(request: Request) { let url = new URL(request.url); - url.searchParams.delete("_data"); + url.searchParams.delete("_routes"); let init: RequestInit = { method: request.method, body: request.body, diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 6705c26914..f9d4ef49cc 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -175,7 +175,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( }; } let headers = new Headers(response.headers); - headers.set("Content-Type", "text/x-turbo"); + headers.set("Content-Type", "text/x-script"); return new Response( encodeViaTurboStream( @@ -296,7 +296,17 @@ async function handleSingleFetchRequest( // network errors that are missing this header let resultHeaders = new Headers(headers); resultHeaders.set("X-Remix-Response", "yes"); - resultHeaders.set("Content-Type", "text/x-turbo"); + + // 304 responses should not have a body + if (status === 304) { + return new Response(null, { status: 304, headers: resultHeaders }); + } + + // We use a less-descriptive `text/x-script` here instead of something like + // `text/x-turbo` to enable compression when deployed via Cloudflare. See: + // - https://github.com/remix-run/remix/issues/9884 + // - https://developers.cloudflare.com/speed/optimization/content/brotli/content-compression/ + resultHeaders.set("Content-Type", "text/x-script"); return new Response( encodeViaTurboStream( @@ -337,6 +347,11 @@ async function handleDocumentRequest( let headers = getDocumentHeaders(build, context); + // 304 responses should not have a body or a content-type + if (context.statusCode === 304) { + return new Response(null, { status: 304, headers }); + } + // Sanitize errors outside of development environments if (context.errors) { Object.values(context.errors).forEach((err) => { @@ -362,7 +377,6 @@ async function handleDocumentRequest( staticHandlerContext: context, criticalCss, serverHandoffString: createServerHandoffString({ - url: context.location.pathname, basename: build.basename, criticalCss, future: build.future, @@ -434,7 +448,6 @@ async function handleDocumentRequest( ...entryContext, staticHandlerContext: context, serverHandoffString: createServerHandoffString({ - url: context.location.pathname, basename: build.basename, future: build.future, isSpaMode: build.isSpaMode, diff --git a/packages/react-router/lib/server-runtime/serverHandoff.ts b/packages/react-router/lib/server-runtime/serverHandoff.ts index a04a17d188..cd0706d4e2 100644 --- a/packages/react-router/lib/server-runtime/serverHandoff.ts +++ b/packages/react-router/lib/server-runtime/serverHandoff.ts @@ -19,7 +19,6 @@ export function createServerHandoffString(serverHandoff: { // we'd end up including duplicate info state?: ValidateShape; criticalCss?: string; - url: string; basename: string | undefined; future: FutureConfig; isSpaMode: boolean; diff --git a/packages/react-router/lib/server-runtime/single-fetch.ts b/packages/react-router/lib/server-runtime/single-fetch.ts index e887563b5b..45f5a11ee1 100644 --- a/packages/react-router/lib/server-runtime/single-fetch.ts +++ b/packages/react-router/lib/server-runtime/single-fetch.ts @@ -44,27 +44,22 @@ export function getSingleFetchDataStrategy({ return async ({ request, matches }: DataStrategyFunctionArgs) => { // Don't call loaders on action data requests if (isActionDataRequest && request.method === "GET") { - return await Promise.all( - matches.map((m) => - m.resolve(async () => ({ type: "data", result: null })) - ) - ); + return {}; } + // Only run opt-in loaders when fine-grained revalidation is enabled + let matchesToLoad = loadRouteIds + ? matches.filter((m) => loadRouteIds.includes(m.route.id)) + : matches; + let results = await Promise.all( - matches.map(async (match) => { - let result = await match.resolve(async (handler) => { - // Only run opt-in loaders when fine-grained revalidation is enabled - let data = - loadRouteIds && !loadRouteIds.includes(match.route.id) - ? null - : await handler(); - return { type: "data", result: data }; - }); - return result; - }) + matchesToLoad.map((match) => match.resolve()) + ); + return results.reduce( + (acc, result, i) => + Object.assign(acc, { [matchesToLoad[i].route.id]: result }), + {} ); - return results; }; }