Skip to content

Commit

Permalink
Allow returning undefined from loaders/actions (part 2) (#12057)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brophdawg11
brophdawg11 authored Oct 3, 2024
1 parent 8fc8239 commit 91a7fdc
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 228 deletions.
7 changes: 7 additions & 0 deletions .changeset/red-feet-shop.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"react-router": minor
---

[REMOVE] Allow returning undefined from loaders/actions part 2

- This is a follow up to #11680 which missed some of the Remix codepaths
180 changes: 55 additions & 125 deletions integration/error-boundary-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,109 +493,73 @@ test.describe("ErrorBoundary", () => {
test.describe("if no error boundary exists in the app", () => {
let NO_ROOT_BOUNDARY_LOADER = "/loader-bad" as const;
let NO_ROOT_BOUNDARY_ACTION = "/action-bad" as const;
let NO_ROOT_BOUNDARY_LOADER_RETURN = "/loader-no-return" as const;
let NO_ROOT_BOUNDARY_ACTION_RETURN = "/action-no-return" as const;

test.beforeAll(async () => {
fixture = await createFixture({
files: {
"app/root.tsx": js`
import { Links, Meta, Outlet, Scripts } from "react-router";
export default function Root() {
return (
<html lang="en">
<head>
<Meta />
<Links />
</head>
<body>
<Outlet />
<Scripts />
</body>
</html>
);
}
`,
import { Links, Meta, Outlet, Scripts } from "react-router";
export default function Root() {
return (
<html lang="en">
<head>
<Meta />
<Links />
</head>
<body>
<Outlet />
<Scripts />
</body>
</html>
);
}
`,

"app/routes/_index.tsx": js`
import { Link, Form } from "react-router";
import { Link, Form } from "react-router";
export default function () {
return (
<div>
<h1>Home</h1>
<Link to="${NO_ROOT_BOUNDARY_LOADER_RETURN}">Loader no return</Link>
<Form method="post">
<button formAction="${NO_ROOT_BOUNDARY_ACTION}" type="submit">
Action go boom
</button>
<button formAction="${NO_ROOT_BOUNDARY_ACTION_RETURN}" type="submit">
Action no return
</button>
</Form>
</div>
)
}
`,
export default function () {
return (
<div>
<h1>Home</h1>
<Form method="post">
<button formAction="${NO_ROOT_BOUNDARY_ACTION}" type="submit">
Action go boom
</button>
</Form>
</div>
)
}
`,

[`app/routes${NO_ROOT_BOUNDARY_LOADER}.jsx`]: js`
export async function loader() {
throw Error("BLARGH");
}
export default function () {
return (
<div>
<h1>Hello</h1>
</div>
)
}
`,
export async function loader() {
throw Error("BLARGH");
}
export default function () {
return (
<div>
<h1>Hello</h1>
</div>
)
}
`,

[`app/routes${NO_ROOT_BOUNDARY_ACTION}.jsx`]: js`
export async function action() {
throw Error("YOOOOOOOO WHAT ARE YOU DOING");
}
export default function () {
return (
<div>
<h1>Goodbye</h1>
</div>
)
}
`,

[`app/routes${NO_ROOT_BOUNDARY_LOADER_RETURN}.jsx`]: js`
import { useLoaderData } from "react-router";
export async function loader() {}
export default function () {
let data = useLoaderData();
return (
<div>
<h1>{data}</h1>
</div>
)
}
`,

[`app/routes${NO_ROOT_BOUNDARY_ACTION_RETURN}.jsx`]: js`
import { useActionData } from "react-router";
export async function action() {}
export default function () {
let data = useActionData();
return (
<div>
<h1>{data}</h1>
</div>
)
}
`,
export async function action() {
throw Error("YOOOOOOOO WHAT ARE YOU DOING");
}
export default function () {
return (
<div>
<h1>Goodbye</h1>
</div>
)
}
`,
},
});
appFixture = await createAppFixture(fixture);
Expand All @@ -618,40 +582,6 @@ test.describe("ErrorBoundary", () => {
await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("bubbles to internal boundary if loader doesn't return (document requests)", async () => {
let res = await fixture.requestDocument(NO_ROOT_BOUNDARY_LOADER_RETURN);
expect(res.status).toBe(500);
expect(await res.text()).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("bubbles to internal boundary if loader doesn't return", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink(NO_ROOT_BOUNDARY_LOADER_RETURN);
await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("bubbles to internal boundary if action doesn't return (document requests)", async () => {
let res = await fixture.requestDocument(NO_ROOT_BOUNDARY_ACTION_RETURN, {
method: "post",
});
expect(res.status).toBe(500);
expect(await res.text()).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});

test("bubbles to internal boundary if action doesn't return", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickSubmitButton(NO_ROOT_BOUNDARY_ACTION_RETURN);
await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`);
expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
});
});
});

Expand Down
88 changes: 88 additions & 0 deletions integration/single-fetch-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ test.describe("single-fetch", () => {
await page.waitForSelector("#error");
expect(urls).toEqual([]);
});

test("returns headers correctly for singular loader and action calls", async () => {
let fixture = await createFixture({
files: {
Expand Down Expand Up @@ -3432,4 +3433,91 @@ test.describe("single-fetch", () => {
}
expect(remixScriptsCount).toBe(4);
});

test("supports loaders that return undefined", async ({ page }) => {
let fixture = await createFixture(
{
files: {
...files,
"app/routes/_index.tsx": js`
import { Link } from "react-router";
export default function () {
return <Link to="/loader">Go to /loader</Link>;
}
`,
"app/routes/loader.tsx": js`
import { useLoaderData } from "react-router";
export async function loader() {}
export default function () {
let data = useLoaderData();
return <h1>{data === undefined? 'It worked!' : 'Error'}</h1>;
}
`,
},
},
ServerMode.Development
);

// Document requests
let res = await fixture.requestDocument("/loader");
expect(res.status).toBe(200);
expect(await res.text()).toMatch("It worked!");

// SPA navigations
let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickLink("/loader");
await page.waitForSelector("h1");
expect(await app.getHtml("h1")).toMatch("It worked!");
});

test("supports actions that return undefined", async ({ page }) => {
let fixture = await createFixture(
{
files: {
...files,
"app/routes/_index.tsx": js`
import { Form } from "react-router";
export default function () {
return (
<Form method="post" action="/action">
<input name="test" />
<button type="submit">Submit</button>
</Form>
);
}
`,
"app/routes/action.tsx": js`
import { useActionData } from "react-router";
export async function action() {}
export default function () {
let data = useActionData();
return <h1>{data === undefined? 'It worked!' : 'Error'}</h1>;
}
`,
},
},
ServerMode.Development
);

// Document requests
let res = await fixture.requestDocument("/action");
expect(res.status).toBe(200);
expect(await res.text()).toMatch("It worked!");

// SPA navigations
let appFixture = await createAppFixture(fixture, ServerMode.Development);
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await app.clickSubmitButton("/action");
await page.waitForSelector("h1");
expect(await app.getHtml("h1")).toMatch("It worked!");
});
});
1 change: 0 additions & 1 deletion packages/react-router/__tests__/router/revalidate-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,6 @@ describe("router.revalidate", () => {
navigation: IDLE_NAVIGATION,
revalidation: "idle",
loaderData: {
root: undefined,
index: "INDEX_DATA*",
},
errors: {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/lib/dom-export/hydrated-router.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function createHydratedRouter(): RemixRouter {
) &&
(route.HydrateFallback || !manifestRoute.hasLoader)
) {
hydrationData.loaderData![routeId] = undefined;
delete hydrationData.loaderData![routeId];
} else if (manifestRoute && !manifestRoute.hasLoader) {
// Since every Remix route gets a `loader` on the client side to load
// the route JS module, we need to add a `null` value to `loaderData`
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/lib/dom/ssr/server.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function ServerRouter({
shouldHydrateRouteLoader(manifestRoute, route, context.isSpaMode) &&
(route.HydrateFallback || !manifestRoute.hasLoader)
) {
context.staticHandlerContext.loaderData[routeId] = undefined;
delete context.staticHandlerContext.loaderData[routeId];
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-router/lib/hooks.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ export function _renderMatches(
let { loaderData, errors } = dataRouterState;
let needsToRunLoader =
match.route.loader &&
loaderData[match.route.id] === undefined &&
!loaderData.hasOwnProperty(match.route.id) &&
(!errors || errors[match.route.id] === undefined);
if (match.route.lazy || needsToRunLoader) {
// We found the first route that's not ready to render (waiting on
Expand Down
Loading

0 comments on commit 91a7fdc

Please sign in to comment.