From fa9e138cf6c01f31eccf3b8aeab800199e82954b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 11:00:45 -0500 Subject: [PATCH 1/3] Reorganize tests --- integration/spa-mode-test.ts | 654 +++++++++++++++++------------------ 1 file changed, 324 insertions(+), 330 deletions(-) diff --git a/integration/spa-mode-test.ts b/integration/spa-mode-test.ts index 657fb0f53e9..ebac558a68c 100644 --- a/integration/spa-mode-test.ts +++ b/integration/spa-mode-test.ts @@ -9,187 +9,15 @@ import type { Fixture, AppFixture } from "./helpers/create-fixture.js"; import { PlaywrightFixture } from "./helpers/playwright-fixture.js"; import { createProject, viteBuild } from "./helpers/vite.js"; -// SSR'd useId value we can assert against pre- and post-hydration -const USE_ID_VALUE = ":R1:"; - test.describe("SPA Mode", () => { let fixture: Fixture; let appFixture: AppFixture; - test.beforeAll(async () => { - fixture = await createFixture({ - compiler: "vite", - spaMode: true, - files: { - "vite.config.ts": js` - import { defineConfig } from "vite"; - import { unstable_vitePlugin as remix } from "@remix-run/dev"; - - export default defineConfig({ - plugins: [remix({ unstable_ssr: false })], - }); - `, - "app/root.tsx": js` - import * as React from "react"; - import { Form, Link, Links, Meta, Outlet, Scripts } from "@remix-run/react"; - - export default function Root() { - let id = React.useId(); - return ( - - - - - - -

Root

-
{id}
- - - - - - ); - } - - export function HydrateFallback() { - const id = React.useId(); - const [hydrated, setHydrated] = React.useState(false); - React.useEffect(() => setHydrated(true), []); - - return ( - - - - - - -

Loading SPA...

-
{id}
- {hydrated ?

Hydrated

: null} - - - - ); - } - `, - "app/routes/_index.tsx": js` - import * as React from "react"; - import { useLoaderData } from "@remix-run/react"; - - export function meta({ data }) { - return [{ - title: "Index Title: " + data - }]; - } - - export async function clientLoader({ request }) { - if (new URL(request.url).searchParams.has('slow')) { - await new Promise(r => setTimeout(r, 500)); - } - return "Index Loader Data"; - } - - export default function Component() { - let data = useLoaderData(); - const [mounted, setMounted] = React.useState(false); - React.useEffect(() => setMounted(true), []); - - return ( - <> -

Index

-

{data}

- {!mounted ?

Unmounted

:

Mounted

} - - ); - } - `, - "app/routes/about.tsx": js` - import { useActionData, useLoaderData } from "@remix-run/react"; - - export function meta({ data }) { - return [{ - title: "About Title: " + data - }]; - } - - export function clientLoader() { - return "About Loader Data"; - } - - export function clientAction() { - return "About Action Data"; - } - - export default function Component() { - let data = useLoaderData(); - let actionData = useActionData(); - - return ( - <> -

About

-

{data}

-

{actionData}

- - ); - } - `, - "app/routes/error.tsx": js` - import { useRouteError } from "@remix-run/react"; - - export async function clientLoader({ serverLoader }) { - await serverLoader(); - return null; - } - - export async function clientAction({ serverAction }) { - await serverAction(); - return null; - } - - export default function Component() { - return

Error

; - } - - export function ErrorBoundary() { - let error = useRouteError(); - return
{error.data}
- } - `, - }, - }); - - appFixture = await createAppFixture(fixture); - }); - - test.afterAll(() => { - appFixture.close(); - }); - - test.describe("builds", () => { - test("errors on server-only exports", async () => { - let cwd = await createProject({ - "vite.config.ts": js` + test.describe("custom builds", () => { + test.describe("build errors", () => { + test("errors on server-only exports", async () => { + let cwd = await createProject({ + "vite.config.ts": js` import { defineConfig } from "vite"; import { unstable_vitePlugin as remix } from "@remix-run/dev"; @@ -197,7 +25,7 @@ test.describe("SPA Mode", () => { plugins: [remix({ unstable_ssr: false })], }); `, - "app/routes/invalid-exports.tsx": String.raw` + "app/routes/invalid-exports.tsx": String.raw` // Invalid exports export function headers() {} export function loader() {} @@ -208,19 +36,19 @@ test.describe("SPA Mode", () => { export function clientAction() {} export default function Component() {} `, + }); + let result = viteBuild({ cwd }); + let stderr = result.stderr.toString("utf8"); + expect(stderr).toMatch( + "SPA Mode: 3 invalid route export(s) in `routes/invalid-exports.tsx`: " + + "`headers`, `loader`, `action`. See https://remix.run/future/spa-mode " + + "for more information." + ); }); - let result = viteBuild({ cwd }); - let stderr = result.stderr.toString("utf8"); - expect(stderr).toMatch( - "SPA Mode: 3 invalid route export(s) in `routes/invalid-exports.tsx`: " + - "`headers`, `loader`, `action`. See https://remix.run/future/spa-mode " + - "for more information." - ); - }); - test("errors on HydrateFallback export from non-root route", async () => { - let cwd = await createProject({ - "vite.config.ts": js` + test("errors on HydrateFallback export from non-root route", async () => { + let cwd = await createProject({ + "vite.config.ts": js` import { defineConfig } from "vite"; import { unstable_vitePlugin as remix } from "@remix-run/dev"; @@ -228,7 +56,7 @@ test.describe("SPA Mode", () => { plugins: [remix({ unstable_ssr: false })], }); `, - "app/routes/invalid-exports.tsx": String.raw` + "app/routes/invalid-exports.tsx": String.raw` // Invalid exports export function HydrateFallback() {} @@ -237,19 +65,19 @@ test.describe("SPA Mode", () => { export function clientAction() {} export default function Component() {} `, + }); + let result = viteBuild({ cwd }); + let stderr = result.stderr.toString("utf8"); + expect(stderr).toMatch( + "SPA Mode: Invalid `HydrateFallback` export found in `routes/invalid-exports.tsx`. " + + "`HydrateFallback` is only permitted on the root route in SPA Mode. " + + "See https://remix.run/future/spa-mode for more information." + ); }); - let result = viteBuild({ cwd }); - let stderr = result.stderr.toString("utf8"); - expect(stderr).toMatch( - "SPA Mode: Invalid `HydrateFallback` export found in `routes/invalid-exports.tsx`. " + - "`HydrateFallback` is only permitted on the root route in SPA Mode. " + - "See https://remix.run/future/spa-mode for more information." - ); - }); - test("errors on a non-200 status from entry.server.tsx", async () => { - let cwd = await createProject({ - "vite.config.ts": js` + test("errors on a non-200 status from entry.server.tsx", async () => { + let cwd = await createProject({ + "vite.config.ts": js` import { defineConfig } from "vite"; import { unstable_vitePlugin as remix } from "@remix-run/dev"; @@ -257,7 +85,7 @@ test.describe("SPA Mode", () => { plugins: [remix({ unstable_ssr: false })], }); `, - "app/entry.server.tsx": js` + "app/entry.server.tsx": js` import { RemixServer } from "@remix-run/react"; import { renderToString } from "react-dom/server"; @@ -276,7 +104,7 @@ test.describe("SPA Mode", () => { }); } `, - "app/root.tsx": js` + "app/root.tsx": js` import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; export default function Root() { @@ -309,19 +137,19 @@ test.describe("SPA Mode", () => { ); } `, + }); + let result = viteBuild({ cwd }); + let stderr = result.stderr.toString("utf8"); + expect(stderr).toMatch( + "SPA Mode: Received a 500 status code from `entry.server.tsx` while " + + "generating the `index.html` file." + ); + expect(stderr).toMatch("

Loading...

"); }); - let result = viteBuild({ cwd }); - let stderr = result.stderr.toString("utf8"); - expect(stderr).toMatch( - "SPA Mode: Received a 500 status code from `entry.server.tsx` while " + - "generating the `index.html` file." - ); - expect(stderr).toMatch("

Loading...

"); - }); - test("errors if you do not include in your root ", async () => { - let cwd = await createProject({ - "vite.config.ts": js` + test("errors if you do not include in your root ", async () => { + let cwd = await createProject({ + "vite.config.ts": js` import { defineConfig } from "vite"; import { unstable_vitePlugin as remix } from "@remix-run/dev"; @@ -329,41 +157,289 @@ test.describe("SPA Mode", () => { plugins: [remix({ unstable_ssr: false })], }); `, - "app/root.tsx": String.raw` + "app/root.tsx": String.raw` export function HydrateFallback() { return

Loading

} `, + }); + let result = viteBuild({ cwd }); + let stderr = result.stderr.toString("utf8"); + expect(stderr).toMatch( + "SPA Mode: Did you forget to include in your `root.tsx` " + + "`HydrateFallback` component? Your `index.html` file cannot hydrate " + + "into a SPA without ``." + ); }); - let result = viteBuild({ cwd }); - let stderr = result.stderr.toString("utf8"); - expect(stderr).toMatch( - "SPA Mode: Did you forget to include in your `root.tsx` " + - "`HydrateFallback` component? Your `index.html` file cannot hydrate " + - "into a SPA without ``." - ); }); - }); - test.describe("javascript disabled", () => { - test.use({ javaScriptEnabled: false }); + test("prepends DOCTYPE to documents if not present", async () => { + let fixture = await createFixture({ + compiler: "vite", + spaMode: true, + files: { + "vite.config.ts": js` + import { defineConfig } from "vite"; + import { unstable_vitePlugin as remix } from "@remix-run/dev"; - test("renders the root HydrateFallback", async ({ page }) => { - let app = new PlaywrightFixture(appFixture, page); - await app.goto("/"); - expect(await page.locator("[data-loading]").textContent()).toBe( - "Loading SPA..." - ); - expect(await page.locator("[data-use-id]").textContent()).toBe( - USE_ID_VALUE - ); - expect(await page.locator("title").textContent()).toBe( - "Index Title: undefined" - ); + export default defineConfig({ + plugins: [remix({ unstable_ssr: false })], + }); + `, + "app/root.tsx": js` + import { Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + return ( + + + +

Root

+ + + + ); + } + + export function HydrateFallback() { + return ( + + + +

Loading SPA...

+ + + + ); + } + `, + }, + }); + let res = await fixture.requestDocument("/"); + expect(await res.text()).toMatch(/^\n/); + }); + + test("does not prepend DOCTYPE if user is not hydrating the document", async () => { + let fixture = await createFixture({ + compiler: "vite", + spaMode: true, + files: { + "vite.config.ts": js` + import { defineConfig } from "vite"; + import { unstable_vitePlugin as remix } from "@remix-run/dev"; + + export default defineConfig({ + plugins: [remix({ unstable_ssr: false })], + }); + `, + "app/root.tsx": js` + import { Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + return ( +
+

Root

+ +
+ ); + } + + export function HydrateFallback() { + return ( +
+

Loading SPA...

+ +
+ ); + } + `, + }, + }); + let res = await fixture.requestDocument("/"); + let html = await res.text(); + expect(html).toMatch(/^
/); + expect(html).not.toMatch(//); }); }); - test.describe("javascript enabled", () => { + test.describe("normal apps", () => { + test.beforeAll(async () => { + fixture = await createFixture({ + compiler: "vite", + spaMode: true, + files: { + "vite.config.ts": js` + import { defineConfig } from "vite"; + import { unstable_vitePlugin as remix } from "@remix-run/dev"; + + export default defineConfig({ + plugins: [remix({ unstable_ssr: false })], + }); + `, + "app/root.tsx": js` + import * as React from "react"; + import { Form, Link, Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + let id = React.useId(); + return ( + + + + + + +

Root

+
{id}
+ + + + + + ); + } + + export function HydrateFallback() { + const id = React.useId(); + const [hydrated, setHydrated] = React.useState(false); + React.useEffect(() => setHydrated(true), []); + + return ( + + + + + + +

Loading SPA...

+
{id}
+ {hydrated ?

Hydrated

: null} + + + + ); + } + `, + "app/routes/_index.tsx": js` + import * as React from "react"; + import { useLoaderData } from "@remix-run/react"; + + export function meta({ data }) { + return [{ + title: "Index Title: " + data + }]; + } + + export async function clientLoader({ request }) { + if (new URL(request.url).searchParams.has('slow')) { + await new Promise(r => setTimeout(r, 500)); + } + return "Index Loader Data"; + } + + export default function Component() { + let data = useLoaderData(); + const [mounted, setMounted] = React.useState(false); + React.useEffect(() => setMounted(true), []); + + return ( + <> +

Index

+

{data}

+ {!mounted ?

Unmounted

:

Mounted

} + + ); + } + `, + "app/routes/about.tsx": js` + import { useActionData, useLoaderData } from "@remix-run/react"; + + export function meta({ data }) { + return [{ + title: "About Title: " + data + }]; + } + + export function clientLoader() { + return "About Loader Data"; + } + + export function clientAction() { + return "About Action Data"; + } + + export default function Component() { + let data = useLoaderData(); + let actionData = useActionData(); + + return ( + <> +

About

+

{data}

+

{actionData}

+ + ); + } + `, + "app/routes/error.tsx": js` + import { useRouteError } from "@remix-run/react"; + + export async function clientLoader({ serverLoader }) { + await serverLoader(); + return null; + } + + export async function clientAction({ serverAction }) { + await serverAction(); + return null; + } + + export default function Component() { + return

Error

; + } + + export function ErrorBoundary() { + let error = useRouteError(); + return
{error.data}
+ } + `, + }, + }); + + appFixture = await createAppFixture(fixture); + }); + + test.afterAll(() => { + appFixture.close(); + }); + + test("renders the root HydrateFallback initially", async ({ page }) => { + let res = await fixture.requestDocument("/"); + let html = await res.text(); + expect(html).toMatch("Index Title: undefined"); + expect(html).toMatch('

Loading SPA...

'); + }); + test("hydrates", async ({ page }) => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); @@ -380,17 +456,23 @@ test.describe("SPA Mode", () => { }); test("hydrates a proper useId value", async ({ page }) => { + // SSR'd useId value we can assert against pre- and post-hydration + let USE_ID_VALUE = ":R1:"; + + // Ensure we SSR a proper useId value + let res = await fixture.requestDocument("/"); + let html = await res.text(); + expect(html).toMatch(`
${USE_ID_VALUE}
`); + + // We should hydrate the same useId value in HydrateFallback let app = new PlaywrightFixture(appFixture, page); await app.goto("/?slow"); - - // We should hydrate the same useId value in HydrateFallback that we - // rendered on the server above await page.waitForSelector("[data-hydrated]"); expect(await page.locator("[data-use-id]").textContent()).toBe( USE_ID_VALUE ); - // Once hydrated, we should get a different useId value from the root component + // Once hydrated, we should get a different useId value from the root Component await page.waitForSelector("[data-route]"); expect(await page.locator("[data-route]").textContent()).toBe("Index"); expect(await page.locator("[data-use-id]").textContent()).not.toBe( @@ -456,93 +538,5 @@ test.describe("SPA Mode", () => { 'Error: You cannot call serverAction() in SPA Mode (routeId: "routes/error")' ); }); - - test("prepends DOCTYPE to documents if not present", async () => { - let fixture = await createFixture({ - compiler: "vite", - spaMode: true, - files: { - "vite.config.ts": js` - import { defineConfig } from "vite"; - import { unstable_vitePlugin as remix } from "@remix-run/dev"; - - export default defineConfig({ - plugins: [remix({ unstable_ssr: false })], - }); - `, - "app/root.tsx": js` - import { Outlet, Scripts } from "@remix-run/react"; - - export default function Root() { - return ( - - - -

Root

- - - - ); - } - - export function HydrateFallback() { - return ( - - - -

Loading SPA...

- - - - ); - } - `, - }, - }); - let res = await fixture.requestDocument("/"); - expect(await res.text()).toMatch(/^\n/); - }); - - test("does not prepend DOCTYPE if user is not hydrating the document", async () => { - let fixture = await createFixture({ - compiler: "vite", - spaMode: true, - files: { - "vite.config.ts": js` - import { defineConfig } from "vite"; - import { unstable_vitePlugin as remix } from "@remix-run/dev"; - - export default defineConfig({ - plugins: [remix({ unstable_ssr: false })], - }); - `, - "app/root.tsx": js` - import { Outlet, Scripts } from "@remix-run/react"; - - export default function Root() { - return ( -
-

Root

- -
- ); - } - - export function HydrateFallback() { - return ( -
-

Loading SPA...

- -
- ); - } - `, - }, - }); - let res = await fixture.requestDocument("/"); - let html = await res.text(); - expect(html).toMatch(/^
/); - expect(html).not.toMatch(//); - }); }); }); From 0abe524225148ba34bccbf1923e857a8ce6ed1cd Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 11:38:48 -0500 Subject: [PATCH 2/3] Only use active matches in meta/links --- integration/spa-mode-test.ts | 52 ++++++++++++++++++++++++++++- packages/remix-react/components.tsx | 49 +++++++++++++++++---------- 2 files changed, 82 insertions(+), 19 deletions(-) diff --git a/integration/spa-mode-test.ts b/integration/spa-mode-test.ts index ebac558a68c..2b004714b27 100644 --- a/integration/spa-mode-test.ts +++ b/integration/spa-mode-test.ts @@ -3,6 +3,7 @@ import { test, expect } from "@playwright/test"; import { createAppFixture, createFixture, + css, js, } from "./helpers/create-fixture.js"; import type { Fixture, AppFixture } from "./helpers/create-fixture.js"; @@ -276,10 +277,33 @@ test.describe("SPA Mode", () => { plugins: [remix({ unstable_ssr: false })], }); `, + "public/styles-root.css": css` + body { + background-color: rgba(255, 0, 0, 0.25); + } + `, + "public/styles-index.css": css` + body { + background-color: rgba(0, 255, 0, 0.25); + } + `, "app/root.tsx": js` import * as React from "react"; import { Form, Link, Links, Meta, Outlet, Scripts } from "@remix-run/react"; + export function meta({ data }) { + return [{ + title: "Root Title" + }]; + } + + export function links() { + return [{ + rel: "stylesheet", + href: "styles-root.css" + }]; + } + export default function Root() { let id = React.useId(); return ( @@ -350,6 +374,13 @@ test.describe("SPA Mode", () => { }]; } + export function links() { + return [{ + rel: "stylesheet", + href: "styles-index.css" + }]; + } + export async function clientLoader({ request }) { if (new URL(request.url).searchParams.has('slow')) { await new Promise(r => setTimeout(r, 500)); @@ -436,10 +467,29 @@ test.describe("SPA Mode", () => { test("renders the root HydrateFallback initially", async ({ page }) => { let res = await fixture.requestDocument("/"); let html = await res.text(); - expect(html).toMatch("Index Title: undefined"); expect(html).toMatch('

Loading SPA...

'); }); + test("does not include Meta/Links from routes below the root", async ({ + page, + }) => { + let res = await fixture.requestDocument("/"); + let html = await res.text(); + expect(html).toMatch("Root Title"); + expect(html).toMatch(''); + expect(html).not.toMatch("Index Title"); + expect(html).not.toMatch("styles-index.css"); + + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await page.waitForSelector("[data-mounted]"); + expect(await page.locator('link[href="styles-index.css"]')).toBeDefined(); + expect(await page.locator("[data-route]").textContent()).toBe("Index"); + expect(await page.locator("title").textContent()).toBe( + "Index Title: Index Loader Data" + ); + }); + test("hydrates", async ({ page }) => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index f37a6fc58dd..f9604f7655e 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -7,6 +7,7 @@ import * as React from "react"; import type { AgnosticDataRouteMatch, UNSAFE_DeferredData as DeferredData, + RouterState, TrackedPromise, UIMatch as UIMatchRR, } from "@remix-run/router"; @@ -267,21 +268,38 @@ export function composeEventHandlers< }; } +// Return the matches actively being displayed: +// - In SPA Mode we only SSR/hydrate the root match, and include all matches +// after hydration. This lets the router handle initial match loads via lazy(). +// - When an error boundary is rendered, we slice off matches up to the +// boundary for / +function getActiveMatches( + matches: RouterState["matches"], + errors: RouterState["errors"], + isSpaMode: boolean +) { + if (isSpaMode && !isHydrated) { + return [matches[0]]; + } + + if (errors) { + let errorIdx = matches.findIndex((m) => errors[m.route.id]); + return matches.slice(0, errorIdx + 1); + } + + return matches; +} + /** * Renders the `` tags for the current routes. * * @see https://remix.run/components/links */ export function Links() { - let { manifest, routeModules, criticalCss } = useRemixContext(); + let { isSpaMode, manifest, routeModules, criticalCss } = useRemixContext(); let { errors, matches: routerMatches } = useDataRouterStateContext(); - let matches = errors - ? routerMatches.slice( - 0, - routerMatches.findIndex((m) => errors![m.route.id]) + 1 - ) - : routerMatches; + let matches = getActiveMatches(routerMatches, errors, isSpaMode); let keyedLinks = React.useMemo( () => getKeyedLinksForMatches(matches, routeModules, manifest), @@ -433,7 +451,7 @@ function PrefetchPageLinksImpl({ * @see https://remix.run/components/meta */ export function Meta() { - let { routeModules } = useRemixContext(); + let { isSpaMode, routeModules } = useRemixContext(); let { errors, matches: routerMatches, @@ -441,12 +459,11 @@ export function Meta() { } = useDataRouterStateContext(); let location = useLocation(); - let _matches: AgnosticDataRouteMatch[] = routerMatches; + let _matches = getActiveMatches(routerMatches, errors, isSpaMode); + let error: any = null; if (errors) { - let errorIdx = routerMatches.findIndex((m) => errors![m.route.id]); - _matches = routerMatches.slice(0, errorIdx + 1); - error = errors[routerMatches[errorIdx].route.id]; + error = errors[_matches[_matches.length - 1].route.id]; } let meta: MetaDescriptor[] = []; @@ -608,14 +625,10 @@ export function Scripts(props: ScriptProps) { let { manifest, serverHandoffString, abortDelay, serializeError, isSpaMode } = useRemixContext(); let { router, static: isStatic, staticContext } = useDataRouterContext(); - let { matches: dontUseTheseMatches } = useDataRouterStateContext(); + let { matches: routerMatches } = useDataRouterStateContext(); let navigation = useNavigation(); - // Use these `matches` instead :) - // In SPA Mode we only want to import root on the critical path, since we - // want the generated HTML file to be able to be hydrated at non-/ paths as - // well. This lets the router handle initial match loads via lazy(). - let matches = isSpaMode ? [dontUseTheseMatches[0]] : dontUseTheseMatches; + let matches = getActiveMatches(routerMatches, null, isSpaMode); React.useEffect(() => { isHydrated = true; From a57838ef9991fc58f16c2ae57b70f6572bfae5ba Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 17 Jan 2024 11:39:11 -0500 Subject: [PATCH 3/3] Add changeset --- .changeset/calm-geckos-refuse.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/calm-geckos-refuse.md diff --git a/.changeset/calm-geckos-refuse.md b/.changeset/calm-geckos-refuse.md new file mode 100644 index 00000000000..03159379839 --- /dev/null +++ b/.changeset/calm-geckos-refuse.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +Only use active matches in meta/links in SPA mode