Fix: Route with server function as loader that throws notFound crashes route on HMR #5763 #5890

DeTuksa · 2025-11-17T18:20:04Z

Resolves: #5763

Updated packages/router-core/src/router.ts to treat 'notFound' like 'error' in invalidate
Added a new test in packages/react-router/tests/router.test.tsx that verifies a route returning a 404 notFound

Summary by CodeRabbit

Bug Fixes
- Routes previously stuck in a "not found" or error state now properly reset and re-run their loaders when invalidated, ensuring correct content or not-found UI is shown.
Tests
- Added regression tests verifying not-found routes re-run loaders and continue to display the appropriate not-found content after invalidation.

coderabbitai · 2025-11-17T18:20:25Z

Walkthrough

This change treats matches with status === 'notFound' as eligible to be marked pending on invalidate, adds a public invalidate method that clears errors and forces reloading of such matches, and adds two tests verifying HMR-filtered and global invalidation re-run loaders that produce notFound() and keep rendering notFoundComponent.

Changes

Cohort / File(s)	Summary
Tests `packages/react-router/tests/router.test.tsx`	Added two regression tests under the "invalidate" suite: one simulates HMR-filtered invalidation re-running a loader that throws `notFound()` and ensuring `notFoundComponent` remains rendered; the other validates global invalidate re-runs a loader returning `notFound()` and continues rendering the `notFoundComponent`.
Invalidation logic `packages/router-core/src/router.ts`	Added a public `invalidate(opts?)` method and updated commit/invalidate logic so cachedMatches exclude `notFound`/`error` exiting matches and so invalidation converts matches with `status === 'notFound'` to `pending` (clearing errors) to force loader re-runs.

Sequence Diagram(s)

sequenceDiagram
    actor Browser
    participant HMR as HMR System
    participant Router
    participant Loader
    participant UI

    Browser->>HMR: Save file (trigger HMR)
    HMR->>Router: Call invalidate(filter?)
    activate Router
    Router->>Router: Inspect matches' statuses
    alt Match status is notFound or error or forced
        Router->>Router: Clear error, mark match as pending
    end
    Router->>Loader: Re-invoke loader for pending match
    activate Loader
    Loader-->>Router: Throws or returns notFound()
    deactivate Loader
    Router->>UI: Render notFoundComponent
    deactivate Router
    UI-->>Browser: Display notFoundComponent

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Focus review on packages/router-core/src/router.ts (invalidate implementation and commit cachedMatches change).
Verify packages/react-router/tests/router.test.tsx accurately exercise HMR filter and global invalidate scenarios.

Possibly related PRs

fix: HMR: only invalidate router if match is existing and not cached #5713 — touches router.invalidate usage and HMR filtering; closely related to invalidate semantics.
fix: route HMR handling #5710 — modifies HMR-driven invalidate calls; intersects with HMR invalidate flow.
refactor(router-core): flatten loadMatches #4961 — changes around notFound handling in router core; related to match lifecycle.

Suggested reviewers

schiller-manuel
birkskyum

Poem

🐰 A tiny hop, a tiny tweak,
NotFound blinked, then found its cheek.
HMR nudged, loaders ran anew,
The rabbit cheers — the route stayed true! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately identifies the main change: fixing a crash when HMR is triggered on routes with loaders that throw notFound().
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue #5763: treating notFound like error in invalidation logic, preventing crashes during HMR, and adding regression tests.
Out of Scope Changes check	✅ Passed	All changes directly relate to fixing the notFound HMR crash. The invalidate method implementation, match caching logic, and regression tests are all within scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68e25a9 and a8f1364.

📒 Files selected for processing (2)

packages/react-router/tests/router.test.tsx (1 hunks)
packages/router-core/src/router.ts (3 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

📚 Learning: 2025-10-08T08:11:47.088Z

Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

packages/react-router/tests/router.test.tsx
packages/router-core/src/router.ts

📚 Learning: 2025-10-14T18:59:33.990Z

Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.

Applied to files:

packages/react-router/tests/router.test.tsx

📚 Learning: 2025-10-01T18:30:26.591Z

Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

packages/router-core/src/router.ts

🧬 Code graph analysis (1)

packages/react-router/tests/router.test.tsx (3)

packages/router-core/src/index.ts (1)

notFound (374-374)

packages/react-router/src/route.tsx (2)

createRootRoute (606-658)

createRoute (331-411)

packages/react-router/src/Match.tsx (1)

Outlet (315-365)

🔇 Additional comments (3)

packages/react-router/tests/router.test.tsx (1)

1379-1480: Regression tests accurately cover HMR notFound() semantics

Both new tests model the reported bug well: a loader that throws/returns notFound() with a route notFoundComponent, then an HMR-style router.invalidate (with and without a filter). They correctly:

Assert that the route-level notFoundComponent renders before and after invalidation.

Verify the loader is actually re-run (call count increments).

Ensure the normal route component never appears.

The setup uses createMemoryHistory, RouterProvider, and act in a way that should be stable and non-flaky. I don’t see any gaps for the regression you’re targeting.

packages/router-core/src/router.ts (2)

2133-2142: Excluding notFound from cachedMatches aligns cache behavior with invalidation

Filtering exitingMatches so that neither status: 'error' nor status: 'notFound' are cached makes the cache consistent with the new invalidate semantics (these states trigger a re-run on invalidation). This avoids reusing stale not-found or error results after an HMR or manual invalidation, while still caching successful matches as before.

2313-2354: invalidate correctly resets error/notFound matches to pending and drives a reload

The new invalidate implementation behaves as needed:

Applies an optional filter across matches, cachedMatches, and pendingMatches, marking matching entries as invalid: true.

For filtered matches, forces status: 'pending' and clears error when forcePending is set or when the current status is 'error' or 'notFound', ensuring loaders are re-run on the next load() (matching the bugfix goal).

Disables view transitions for this cycle (shouldViewTransition = false), which is appropriate for HMR-style invalidations.

Returns the load({ sync }) promise so callers can await completion.

This matches the documented behavior and the new tests, and it keeps the matchRoutes reuse logic intact by updating state before the next beforeLoad().

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

nx-cloud · 2025-11-18T00:32:14Z

View your CI Pipeline Execution ↗ for commit c4f892b

Command	Status	Duration	Result
`nx affected --targets=test:eslint,test:unit,tes...`	⛔ Cancelled	55s	View ↗

☁️ Nx Cloud last updated this comment at 2025-11-18 13:29:38 UTC

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cd2f15 and 68e25a9.

📒 Files selected for processing (2)

packages/react-router/tests/router.test.tsx (1 hunks)
packages/router-core/src/router.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

packages/react-router/tests/router.test.tsx

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

📚 Learning: 2025-10-01T18:30:26.591Z

Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

packages/router-core/src/router.ts

📚 Learning: 2025-10-08T08:11:47.088Z

Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

packages/router-core/src/router.ts

packages/router-core/src/router.ts

…ute-with-server

birkskyum · 2025-11-18T12:38:30Z

@DeTuksa , can you add a similar test in the solid-router?

DeTuksa · 2025-11-18T13:07:15Z

Sure @birkskyum😀

I added these cases:

it('re-runs loaders that throw notFound() when invalidated via HMR filter', async () => {
    const history = createMemoryHistory({
      initialEntries: ['/hmr-not-found'],
    })
    const loader = vi.fn(() => {
      throw notFound()
    })

    const rootRoute = createRootRoute({
      component: () => <Outlet />,
    })

    const hmrRoute = createRoute({
      getParentRoute: () => rootRoute,
      path: '/hmr-not-found',
      loader,
      component: () => <div data-testid="hmr-route">Route</div>,
      notFoundComponent: () => (
        <div data-testid="hmr-route-not-found">Route Not Found</div>
      ),
    })

    const router = createRouter({
      routeTree: rootRoute.addChildren([hmrRoute]),
      history,
    })

    render(() => <RouterProvider router={router} />)
    await router.load()

    await screen.findByTestId('hmr-route-not-found')
    const initialCalls = loader.mock.calls.length
    expect(initialCalls).toBeGreaterThan(0)

    await router.invalidate({
      filter: (match) => match.routeId === hmrRoute.id,
    })

    await waitFor(() => expect(loader).toHaveBeenCalledTimes(initialCalls + 1))
    await screen.findByTestId('hmr-route-not-found')
    expect(screen.queryByTestId('hmr-route')).not.toBeInTheDocument()
  })

  it('keeps rendering a route notFoundComponent when loader returns notFound() after invalidate', async () => {
    const history = createMemoryHistory({
      initialEntries: ['/loader-not-found'],
    })
    const loader = vi.fn(() => notFound())

    const rootRoute = createRootRoute({
      component: () => <Outlet />,
    })

    const loaderRoute = createRoute({
      getParentRoute: () => rootRoute,
      path: '/loader-not-found',
      loader,
      component: () => <div data-testid="loader-route">Route</div>,
      notFoundComponent: () => (
        <div data-testid="loader-not-found-component">Route Not Found</div>
      ),
    })

    const router = createRouter({
      routeTree: rootRoute.addChildren([loaderRoute]),
      history,
    })

    render(() => <RouterProvider router={router} />)
    await router.load()

    await screen.findByTestId('loader-not-found-component')
    const initialCalls = loader.mock.calls.length
    expect(initialCalls).toBeGreaterThan(0)

    await router.invalidate()

    await waitFor(() => expect(loader).toHaveBeenCalledTimes(initialCalls + 1))
    await screen.findByTestId('loader-not-found-component')
    expect(screen.queryByTestId('loader-route')).not.toBeInTheDocument()
  })

fix: TanStack#5763

3cd2f15

github-actions bot added package: react-router package: router-core labels Nov 17, 2025

ci: apply automated fixes

68e25a9

coderabbitai bot reviewed Nov 18, 2025

View reviewed changes

packages/router-core/src/router.ts Show resolved Hide resolved

DeTuksa added 2 commits November 18, 2025 10:55

chore: added docstrings and resolved potential issue

2794cf0

Merge branch 'route-with-server' of github.com:DeTuksa/router into ro…

a8f1364

…ute-with-server

birkskyum added the missing-solid-version label Nov 18, 2025

ci: apply automated fixes

abe3b86

chore: added similar test to solid-router in solid-router package

c4f892b

github-actions bot added the package: solid-router label Nov 18, 2025

birkskyum removed the missing-solid-version label Nov 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix: Route with server function as loader that throws notFound crashes route on HMR #5763 #5890

Fix: Route with server function as loader that throws notFound crashes route on HMR #5763 #5890

DeTuksa commented Nov 17, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 17, 2025 •

edited

Loading

Uh oh!

nx-cloud bot commented Nov 18, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

birkskyum commented Nov 18, 2025

Uh oh!

DeTuksa commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Fix: Route with server function as loader that throws notFound crashes route on HMR #5763 #5890

Are you sure you want to change the base?

Fix: Route with server function as loader that throws notFound crashes route on HMR #5763 #5890

Conversation

DeTuksa commented Nov 17, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

nx-cloud bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

birkskyum commented Nov 18, 2025

Uh oh!

DeTuksa commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DeTuksa commented Nov 17, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 17, 2025 •

edited

Loading

nx-cloud bot commented Nov 18, 2025 •

edited

Loading