Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw an ErrorResponse instead of just an Error for route with no action #7423

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wicked-avocados-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/react": patch
---

Throw a semantically correct 405 `ErrorResponse` instead of just an `Error` when submitting to a route without an `action`
28 changes: 28 additions & 0 deletions integration/action-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ test.describe("actions", () => {
return <div id="${REDIRECT_TARGET}">${PAGE_TEXT}</div>
}
`,

"app/routes/no-action.tsx": js`
import { Form } from "@remix-run/react";

export default function Component() {
return (
<Form method="post">
<button type="submit">Submit without action</button>
</Form>
);
}
`,
},
});

Expand Down Expand Up @@ -145,6 +157,22 @@ test.describe("actions", () => {
await page.waitForSelector(`#text:has-text("${SUBMITTED_VALUE}")`);
});

test("throws a 405 when no action exists", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/no-action`);
await page.click("button[type=submit]");
await page.waitForSelector(`h1:has-text("405 Method Not Allowed")`);
expect(logs.length).toBe(2);
expect(logs[0]).toMatch('Route "routes/no-action" does not have an action');
// logs[1] is the raw ErrorResponse instance from the boundary but playwright
// seems to just log the name of the constructor, which in the minified code
// is meaningless so we don't bother asserting

// The rest of the tests in this suite assert no logs, so clear this out to
// avoid failures in afterEach
logs = [];
});

test("properly encodes form data for request.text() usage", async ({
page,
}) => {
Expand Down
2 changes: 1 addition & 1 deletion integration/resource-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ test.describe("loader in an app", async () => {
await app.goto("/");
await app.clickSubmitButton("/no-action");
let html = await app.getHtml();
expect(html).toMatch("Application Error");
expect(html).toMatch("405 Method Not Allowed");
expect(logs[0]).toContain(
'Route "routes/no-action" does not have an action'
);
Expand Down
5 changes: 4 additions & 1 deletion packages/remix-react/routes.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from "react";
import { UNSAFE_ErrorResponseImpl as ErrorResponse } from "@remix-run/router";
import type {
DataRouteObject,
ShouldRevalidateFunction,
Expand Down Expand Up @@ -140,7 +141,9 @@ export function createClientRoutes(
`Route "${route.id}" does not have an action, but you are trying ` +
`to submit to it. To fix this, please add an \`action\` function to the route`;
console.error(msg);
return Promise.reject(new Error(msg));
return Promise.reject(
new ErrorResponse(405, "Method Not Allowed", new Error(msg), true)
);
}

return fetchServerHandler(request, route);
Expand Down