Skip to content

Commit

Permalink
Add proper 404 error for missing loader
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Apr 14, 2023
1 parent b7683f1 commit a11f18b
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/fetcher-404.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Ensure proper 404 error on `fetcher.load` call to a route without a `loader`
160 changes: 156 additions & 4 deletions packages/router/__tests__/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10279,6 +10279,150 @@ describe("a router", () => {
await F.actions.index.resolve("INDEX ACTION");
expect(t.router.getFetcher(key).data).toBe("INDEX ACTION");
});

it("throws a 404 ErrorResponse without ?index and parent route has no loader", async () => {
let t = setup({
routes: [
{
id: "parent",
path: "parent",
children: [
{
id: "index",
index: true,
loader: true,
},
],
},
],
initialEntries: ["/parent"],
hydrationData: { loaderData: { index: "INDEX" } },
});

let key = "KEY";
await t.fetch("/parent");
expect(t.router.state.errors).toMatchInlineSnapshot(`
{
"parent": ErrorResponse {
"data": "Error: No route matches URL "/parent"",
"error": [Error: No route matches URL "/parent"],
"internal": true,
"status": 404,
"statusText": "Not Found",
},
}
`);
expect(t.router.getFetcher(key).data).toBe(undefined);
});

it("throws a 404 ErrorResponse with ?index and index route has no loader", async () => {
let t = setup({
routes: [
{
id: "parent",
path: "parent",
loader: true,
children: [
{
id: "index",
index: true,
},
],
},
],
initialEntries: ["/parent"],
hydrationData: { loaderData: { parent: "PARENT" } },
});

let key = "KEY";
await t.fetch("/parent?index");
expect(t.router.state.errors).toMatchInlineSnapshot(`
{
"parent": ErrorResponse {
"data": "Error: No route matches URL "/parent?index"",
"error": [Error: No route matches URL "/parent?index"],
"internal": true,
"status": 404,
"statusText": "Not Found",
},
}
`);
expect(t.router.getFetcher(key).data).toBe(undefined);
});

it("throws a 405 ErrorResponse without ?index and parent route has no action", async () => {
let t = setup({
routes: [
{
id: "parent",
path: "parent",
children: [
{
id: "index",
index: true,
action: true,
},
],
},
],
initialEntries: ["/parent"],
});

let key = "KEY";
await t.fetch("/parent", {
formMethod: "post",
formData: createFormData({}),
});
expect(t.router.state.errors).toMatchInlineSnapshot(`
{
"parent": ErrorResponse {
"data": "Error: You made a POST request to "/parent" but did not provide an \`action\` for route "parent", so there is no way to handle the request.",
"error": [Error: You made a POST request to "/parent" but did not provide an \`action\` for route "parent", so there is no way to handle the request.],
"internal": true,
"status": 405,
"statusText": "Method Not Allowed",
},
}
`);
expect(t.router.getFetcher(key).data).toBe(undefined);
});

it("throws a 405 ErrorResponse with ?index and index route has no action", async () => {
let t = setup({
routes: [
{
id: "parent",
path: "parent",
action: true,
children: [
{
id: "index",
index: true,
},
],
},
],
initialEntries: ["/parent"],
});

let key = "KEY";
await t.fetch("/parent?index", {
formMethod: "post",
formData: createFormData({}),
});
expect(t.router.state.errors).toMatchInlineSnapshot(`
{
"parent": ErrorResponse {
"data": "Error: You made a POST request to "/parent?index" but did not provide an \`action\` for route "parent", so there is no way to handle the request.",
"error": [Error: You made a POST request to "/parent?index" but did not provide an \`action\` for route "parent", so there is no way to handle the request.],
"internal": true,
"status": 405,
"statusText": "Method Not Allowed",
},
}
`);
expect(t.router.getFetcher(key).data).toBe(undefined);
});
});
});

Expand Down Expand Up @@ -15443,12 +15587,20 @@ describe("a router", () => {
expect(currentRouter.state.loaderData).toEqual({
root: "ROOT*",
});
// Fetcher should have been revalidated but thrown an errow since the
// Fetcher should have been revalidated but throw an error since the
// loader was removed
expect(currentRouter.state.fetchers.get("key")?.data).toBe(undefined);
expect(currentRouter.state.errors).toEqual({
root: new Error('Could not find the loader to run on the "foo" route'),
});
expect(currentRouter.state.errors).toMatchInlineSnapshot(`
{
"root": ErrorResponse {
"data": "Error: No route matches URL "/foo"",
"error": [Error: No route matches URL "/foo"],
"internal": true,
"status": 404,
"statusText": "Not Found",
},
}
`);
});

it("should retain existing routes until revalidation completes on route removal (fetch)", async () => {
Expand Down
15 changes: 9 additions & 6 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3425,9 +3425,11 @@ async function callLoaderOrAction(
// previously-lazy-loaded routes
result = await runHandler(handler);
} else if (type === "action") {
let url = new URL(request.url);
let pathname = url.pathname + url.search;
throw getInternalRouterError(405, {
method: request.method,
pathname: new URL(request.url).pathname,
pathname,
routeId: match.route.id,
});
} else {
Expand All @@ -3436,12 +3438,13 @@ async function callLoaderOrAction(
return { type: ResultType.data, data: undefined };
}
}
} else if (!handler) {
let url = new URL(request.url);
let pathname = url.pathname + url.search;
throw getInternalRouterError(404, {
pathname,
});
} else {
invariant<Function>(
handler,
`Could not find the ${type} to run on the "${match.route.id}" route`
);

result = await runHandler(handler);
}

Expand Down

0 comments on commit a11f18b

Please sign in to comment.