Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@brophdawg11
brophdawg11 committed Jan 8, 2024
1 parent b80ca86 commit 525e424
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
49 changes: 43 additions & 6 deletions integration/spa-mode-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ 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;
Expand All @@ -26,9 +29,11 @@ test.describe("SPA Mode", () => {
});
`,
"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 (
<html lang="en">
<head>
Expand All @@ -37,6 +42,7 @@ test.describe("SPA Mode", () => {
</head>
<body>
<h1 data-root>Root</h1>
<pre data-use-id>{id}</pre>
<nav>
<Link to="/about">/about</Link>
<br/>
Expand Down Expand Up @@ -66,6 +72,10 @@ test.describe("SPA Mode", () => {
}
export function HydrateFallback() {
const id = React.useId();
const [hydrated, setHydrated] = React.useState(false);
React.useEffect(() => setHydrated(true), []);
return (
<html lang="en">
<head>
Expand All @@ -74,14 +84,16 @@ test.describe("SPA Mode", () => {
</head>
<body>
<h1 data-loading>Loading SPA...</h1>
<pre data-use-id>{id}</pre>
{hydrated ? <h3 data-hydrated>Hydrated</h3> : null}
<Scripts />
</body>
</html>
);
}
`,
`,
"app/routes/_index.tsx": js`
import { useState, useEffect } from "react";
import * as React from "react";
import { useLoaderData } from "@remix-run/react";
export function meta({ data }) {
Expand All @@ -90,14 +102,17 @@ test.describe("SPA Mode", () => {
}];
}
export function clientLoader() {
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] = useState(false);
useEffect(() => setMounted(true), []);
const [mounted, setMounted] = React.useState(false);
React.useEffect(() => setMounted(true), []);
return (
<>
Expand Down Expand Up @@ -159,7 +174,7 @@ test.describe("SPA Mode", () => {
let error = useRouteError();
return <pre data-error>{error.data}</pre>
}
`,
`,
},
});

Expand Down Expand Up @@ -241,6 +256,9 @@ test.describe("SPA Mode", () => {
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"
);
Expand All @@ -263,6 +281,25 @@ test.describe("SPA Mode", () => {
);
});

test("hydrates a proper useId value", async ({ page }) => {
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
await page.waitForSelector("[data-route]");
expect(await page.locator("[data-route]").textContent()).toBe("Index");
expect(await page.locator("[data-use-id]").textContent()).not.toBe(
USE_ID_VALUE
);
});

test("navigates and calls loaders", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
Expand Down
5 changes: 3 additions & 2 deletions packages/remix-dev/vite/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import type * as Vite from "vite";
import { type BinaryLike, createHash } from "node:crypto";
import * as path from "node:path";
import * as url from "node:url";
import * as fse from "fs-extra";
import babel from "@babel/core";
import {
Expand Down Expand Up @@ -419,7 +420,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
...resolveServerBuildConfig(),
};

// Log warning
// Log warning for incompatible vite config flags
if (isSpaMode && unstable_serverBundles) {
console.warn(
colors.yellow(
Expand Down Expand Up @@ -1517,7 +1518,7 @@ async function handleSpaMode(
// proper HydrateFallback ... or not! Maybe they have a static landing page
// generated from routes/_index.tsx.
let serverBuildPath = path.join(serverBuildDirectoryPath, serverBuildFile);
let build = await import(`file://${serverBuildPath}`);
let build = await import(url.pathToFileURL(serverBuildPath).toString());
let { createRequestHandler: createHandler } = await import("@remix-run/node");
let handler = createHandler(build, viteConfig.mode);
let response = await handler(new Request("http://localhost/"));
Expand Down
10 changes: 5 additions & 5 deletions packages/remix-react/routes.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ export function createServerRoutes(
routesByParentId: Record<
string,
Omit<EntryRoute, "children">[]
> = groupRoutesByParentId(manifest)
> = groupRoutesByParentId(manifest),
spaModeLazyPromise = Promise.resolve({ Component: () => null })
): DataRouteObject[] {
return (routesByParentId[parentId] || []).map((route) => {
let routeModule = routeModules[route.id];
Expand Down Expand Up @@ -108,9 +109,7 @@ export function createServerRoutes(
// implementation here though - just need a `lazy` prop to tell the RR
// rendering where to stop
lazy:
isSpaMode && route.id !== "root"
? () => Promise.resolve({ Component: () => null })
: undefined,
isSpaMode && route.id !== "root" ? () => spaModeLazyPromise : undefined,
// For partial hydration rendering, we need to indicate when the route
// has a loader/clientLoader, but it won't ever be called during the static
// render, so just give it a no-op function so we can render down to the
Expand All @@ -126,7 +125,8 @@ export function createServerRoutes(
future,
isSpaMode,
route.id,
routesByParentId
routesByParentId,
spaModeLazyPromise
);
if (children.length > 0) dataRoute.children = children;
return dataRoute;
Expand Down

0 comments on commit 525e424

Please sign in to comment.