From 992a3d29c461d79d86e2324a0b03ad793d08a1aa Mon Sep 17 00:00:00 2001 From: Dmitriy Cherchenko Date: Sat, 7 Dec 2024 17:02:50 -0600 Subject: [PATCH 1/4] bug repro: demonstrates the TypeError Notice the "Cannot read properties of undefined" --- .../__tests__/router/lazy-discovery-test.ts | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/packages/react-router/__tests__/router/lazy-discovery-test.ts b/packages/react-router/__tests__/router/lazy-discovery-test.ts index 856301b611..ce22d6b81d 100644 --- a/packages/react-router/__tests__/router/lazy-discovery-test.ts +++ b/packages/react-router/__tests__/router/lazy-discovery-test.ts @@ -2103,6 +2103,81 @@ describe("Lazy Route Discovery (Fog of War)", () => { expect(router.state.matches.map((m) => m.route.id)).toEqual(["a", "b"]); }); + it("handles errors thrown from patchRoutesOnNavigation() when there are no partial matches (GET navigation)", async () => { + router = createRouter({ + history: createMemoryHistory(), + routes: [ + { + id: "a", + path: "a", + }, + ], + async patchRoutesOnNavigation({ patch }) { + await tick(); + throw new Error("broke!"); + patch("b", [ + { + id: "b", + path: "b", + loader() { + return "B"; + }, + }, + ]); + }, + }); + + await router.navigate("/b"); + expect(router.state).toMatchObject({ + matches: [], + location: { pathname: "/b" }, + actionData: null, + loaderData: {}, + errors: { + a: new Error("broke!"), + }, + }); + }); + + it("handles errors thrown from patchRoutesOnNavigation() when there are no partial matches (POST navigation)", async () => { + router = createRouter({ + history: createMemoryHistory(), + routes: [ + { + id: "a", + path: "a", + }, + ], + async patchRoutesOnNavigation({ patch }) { + await tick(); + throw new Error("broke!"); + patch("b", [ + { + id: "b", + path: "b", + action() { + return "B"; + }, + }, + ]); + }, + }); + + await router.navigate("/b", { + formMethod: "POST", + formData: createFormData({}), + }); + expect(router.state).toMatchObject({ + matches: [], + location: { pathname: "/b" }, + actionData: null, + loaderData: {}, + errors: { + a: new Error("broke!"), + }, + }); + }); + it("bubbles errors thrown from patchRoutesOnNavigation() during hydration", async () => { router = createRouter({ history: createMemoryHistory({ From 0df9b38b49f702f4760917391aa1ab3f9eb53cef Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 19 Aug 2025 12:42:34 -0400 Subject: [PATCH 2/4] Fix --- .changeset/real-rules-compare.md | 5 +++++ .../__tests__/router/lazy-discovery-test.ts | 18 ------------------ packages/react-router/lib/router/router.ts | 10 ++++++---- 3 files changed, 11 insertions(+), 22 deletions(-) create mode 100644 .changeset/real-rules-compare.md diff --git a/.changeset/real-rules-compare.md b/.changeset/real-rules-compare.md new file mode 100644 index 0000000000..0ff7047f04 --- /dev/null +++ b/.changeset/real-rules-compare.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix `TypeError` if you throw from `patchRoutesOnNavigation` when no partial matches exist diff --git a/packages/react-router/__tests__/router/lazy-discovery-test.ts b/packages/react-router/__tests__/router/lazy-discovery-test.ts index ce22d6b81d..e7f1cfa02e 100644 --- a/packages/react-router/__tests__/router/lazy-discovery-test.ts +++ b/packages/react-router/__tests__/router/lazy-discovery-test.ts @@ -2115,15 +2115,6 @@ describe("Lazy Route Discovery (Fog of War)", () => { async patchRoutesOnNavigation({ patch }) { await tick(); throw new Error("broke!"); - patch("b", [ - { - id: "b", - path: "b", - loader() { - return "B"; - }, - }, - ]); }, }); @@ -2151,15 +2142,6 @@ describe("Lazy Route Discovery (Fog of War)", () => { async patchRoutesOnNavigation({ patch }) { await tick(); throw new Error("broke!"); - patch("b", [ - { - id: "b", - path: "b", - action() { - return "B"; - }, - }, - ]); }, }); diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index c9e9809c96..124538e330 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -1802,8 +1802,9 @@ export function createRouter(init: RouterInit): Router { if (discoverResult.type === "aborted") { return { shortCircuited: true }; } else if (discoverResult.type === "error") { - let boundaryId = findNearestBoundary(discoverResult.partialMatches) - .route.id; + let boundaryId = + findNearestBoundary(discoverResult.partialMatches)?.route.id ?? + getShortCircuitMatches(dataRoutes).route.id; return { matches: discoverResult.partialMatches, pendingActionResult: [ @@ -1996,8 +1997,9 @@ export function createRouter(init: RouterInit): Router { if (discoverResult.type === "aborted") { return { shortCircuited: true }; } else if (discoverResult.type === "error") { - let boundaryId = findNearestBoundary(discoverResult.partialMatches) - .route.id; + let boundaryId = + findNearestBoundary(discoverResult.partialMatches)?.route.id ?? + getShortCircuitMatches(dataRoutes).route.id; return { matches: discoverResult.partialMatches, loaderData: {}, From a88612b9f6e2b745a3cc740c0eb93c5a5de40950 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 19 Aug 2025 13:02:10 -0400 Subject: [PATCH 3/4] Updates --- .../__tests__/router/lazy-discovery-test.ts | 32 +++++++++++++++++-- packages/react-router/lib/router/router.ts | 30 +++++++++++++++-- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/react-router/__tests__/router/lazy-discovery-test.ts b/packages/react-router/__tests__/router/lazy-discovery-test.ts index e7f1cfa02e..3700f3886f 100644 --- a/packages/react-router/__tests__/router/lazy-discovery-test.ts +++ b/packages/react-router/__tests__/router/lazy-discovery-test.ts @@ -2120,7 +2120,21 @@ describe("Lazy Route Discovery (Fog of War)", () => { await router.navigate("/b"); expect(router.state).toMatchObject({ - matches: [], + // A bit odd but this is a result of our best attempt to display some form + // of error UI to the user - follows the same logic we use on 404s + matches: [ + { + params: {}, + pathname: "", + pathnameBase: "", + route: { + children: undefined, + hasErrorBoundary: false, + id: "a", + path: "a", + }, + }, + ], location: { pathname: "/b" }, actionData: null, loaderData: {}, @@ -2150,7 +2164,21 @@ describe("Lazy Route Discovery (Fog of War)", () => { formData: createFormData({}), }); expect(router.state).toMatchObject({ - matches: [], + // A bit odd but this is a result of our best attempt to display some form + // of error UI to the user - follows the same logic we use on 404s + matches: [ + { + params: {}, + pathname: "", + pathnameBase: "", + route: { + children: undefined, + hasErrorBoundary: false, + id: "a", + path: "a", + }, + }, + ], location: { pathname: "/b" }, actionData: null, loaderData: {}, diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 124538e330..aedee720ae 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -1802,9 +1802,22 @@ export function createRouter(init: RouterInit): Router { if (discoverResult.type === "aborted") { return { shortCircuited: true }; } else if (discoverResult.type === "error") { - let boundaryId = - findNearestBoundary(discoverResult.partialMatches)?.route.id ?? - getShortCircuitMatches(dataRoutes).route.id; + if (discoverResult.partialMatches.length === 0) { + let { matches, route } = getShortCircuitMatches(dataRoutes); + return { + matches, + pendingActionResult: [ + route.id, + { + type: ResultType.error, + error: discoverResult.error, + }, + ], + }; + } + + let boundaryId = findNearestBoundary(discoverResult.partialMatches) + ?.route.id; return { matches: discoverResult.partialMatches, pendingActionResult: [ @@ -1997,6 +2010,17 @@ export function createRouter(init: RouterInit): Router { if (discoverResult.type === "aborted") { return { shortCircuited: true }; } else if (discoverResult.type === "error") { + if (discoverResult.partialMatches.length === 0) { + let { matches, route } = getShortCircuitMatches(dataRoutes); + return { + matches, + loaderData: {}, + errors: { + [route.id]: discoverResult.error, + }, + }; + } + let boundaryId = findNearestBoundary(discoverResult.partialMatches)?.route.id ?? getShortCircuitMatches(dataRoutes).route.id; From 1fc15a39c7339ce8a4229c52d71e6821c9863289 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 19 Aug 2025 13:02:58 -0400 Subject: [PATCH 4/4] Updates --- packages/react-router/lib/router/router.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index aedee720ae..dc85b2e2b1 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -1817,7 +1817,7 @@ export function createRouter(init: RouterInit): Router { } let boundaryId = findNearestBoundary(discoverResult.partialMatches) - ?.route.id; + .route.id; return { matches: discoverResult.partialMatches, pendingActionResult: [ @@ -2021,9 +2021,8 @@ export function createRouter(init: RouterInit): Router { }; } - let boundaryId = - findNearestBoundary(discoverResult.partialMatches)?.route.id ?? - getShortCircuitMatches(dataRoutes).route.id; + let boundaryId = findNearestBoundary(discoverResult.partialMatches) + .route.id; return { matches: discoverResult.partialMatches, loaderData: {},