fix(react-router): HMR not updating inline arrow function components #6197
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds an Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-12-06T15:03:07.223ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-10-14T18:59:33.990ZApplied to files:
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. Comment |
|
View your CI Pipeline Execution ↗ for commit 4b3f06e
☁️ Nx Cloud last updated this comment at |
Contributor
|
how often is the new component then rendered then after invalidation? |
longfellowone
force-pushed
the
fix/hmr-inline-arrow-components
branch
2 times, most recently
from
345acb4 to
2add982
Compare
Contributor
Author
|
The component renders exactly once after I've added two render count verification tests that prove this:
The tests track render calls across the component swap and verify only 1 render occurs after You can see the test output in CI or run locally: pnpm test:unit --run -t "render count verification" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-router/src/Match.tsxpackages/react-router/tests/router.test.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/react-router/tests/router.test.tsxpackages/react-router/src/Match.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/react-router/tests/router.test.tsxpackages/react-router/src/Match.tsx
🧠 Learnings (5)
📓 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
📚 Learning: 2025-12-06T15:03:07.223Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Applies to **/*.{js,ts,tsx} : Implement ESLint rules for router best practices using the ESLint plugin router
Applied to files:
packages/react-router/tests/router.test.tsx
📚 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/react-router/tests/router.test.tsx
🔇 Additional comments (2)
packages/react-router/src/Match.tsx (1)
213-213: LGTM! Critical fix for HMR with inline arrow function components.Including
invalidin theuseRouterStateselector ensures React detects state changes whenrouter.invalidate()is called, triggering the necessary re-render to display updated components during HMR.packages/react-router/tests/router.test.tsx (1)
1490-1551: LGTM! Excellent regression test for HMR with inline arrow functions.This test correctly simulates the HMR scenario by swapping the component reference and calling
invalidate(), then verifying the updated component renders. The comprehensive documentation explains the issue and test strategy clearly.
Comment on lines
1604
to
1609
| console.log('[No loader] Initial renders:', initialCallCount) | ||
| console.log('[No loader] Renders after invalidate:', rendersAfterInvalidate) | ||
| console.log('[No loader] Render calls:', renderTracker.mock.calls) | ||
|
|
||
| // We expect exactly 1 render to pick up new component | ||
| expect(rendersAfterInvalidate).toBe(1) |
Contributor
There was a problem hiding this comment.
Remove console.log statements from test code.
The debugging console.log statements (lines 1604-1606) should be removed to keep test output clean.
🔎 Proposed fix
- // Log for debugging
- console.log('[No loader] Initial renders:', initialCallCount)
- console.log('[No loader] Renders after invalidate:', rendersAfterInvalidate)
- console.log('[No loader] Render calls:', renderTracker.mock.calls)
-
// We expect exactly 1 render to pick up new component
expect(rendersAfterInvalidate).toBe(1)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/react-router/tests/router.test.tsx around lines 1604 to 1609, remove
the three debugging console.log statements on lines 1604–1606 (the '[No loader]
Initial renders', '[No loader] Renders after invalidate', and '[No loader]
Render calls' logs) so the test output remains clean and keep the subsequent
expectation (expect(rendersAfterInvalidate).toBe(1)) intact; run the test suite
to confirm no regressions.
Comment on lines
1665
to
1674
| console.log('[With loader] Initial renders:', initialCallCount) | ||
| console.log('[With loader] Renders after invalidate:', rendersAfterInvalidate) | ||
| console.log('[With loader] Loader calls after invalidate:', loaderCallsAfterInvalidate) | ||
| console.log('[With loader] Render calls:', renderTracker.mock.calls) | ||
|
|
||
| // Loader should be called once | ||
| expect(loaderCallsAfterInvalidate).toBe(1) | ||
| // Component should render at most 2 times (once for invalidation, possibly once for load complete) | ||
| expect(rendersAfterInvalidate).toBeGreaterThanOrEqual(1) | ||
| expect(rendersAfterInvalidate).toBeLessThanOrEqual(2) |
Contributor
There was a problem hiding this comment.
Remove console.log statements from test code.
The debugging console.log statements (lines 1665-1668) should be removed to keep test output clean. The render count assertions (1-2 renders with async loader) are reasonable given the async loading lifecycle.
🔎 Proposed fix
- console.log('[With loader] Initial renders:', initialCallCount)
- console.log('[With loader] Renders after invalidate:', rendersAfterInvalidate)
- console.log('[With loader] Loader calls after invalidate:', loaderCallsAfterInvalidate)
- console.log('[With loader] Render calls:', renderTracker.mock.calls)
-
// Loader should be called once
expect(loaderCallsAfterInvalidate).toBe(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log('[With loader] Initial renders:', initialCallCount) | |
| console.log('[With loader] Renders after invalidate:', rendersAfterInvalidate) | |
| console.log('[With loader] Loader calls after invalidate:', loaderCallsAfterInvalidate) | |
| console.log('[With loader] Render calls:', renderTracker.mock.calls) | |
| // Loader should be called once | |
| expect(loaderCallsAfterInvalidate).toBe(1) | |
| // Component should render at most 2 times (once for invalidation, possibly once for load complete) | |
| expect(rendersAfterInvalidate).toBeGreaterThanOrEqual(1) | |
| expect(rendersAfterInvalidate).toBeLessThanOrEqual(2) | |
| // Loader should be called once | |
| expect(loaderCallsAfterInvalidate).toBe(1) | |
| // Component should render at most 2 times (once for invalidation, possibly once for load complete) | |
| expect(rendersAfterInvalidate).toBeGreaterThanOrEqual(1) | |
| expect(rendersAfterInvalidate).toBeLessThanOrEqual(2) |
🤖 Prompt for AI Agents
In packages/react-router/tests/router.test.tsx around lines 1665 to 1674, remove
the four console.log debugging lines that print render/loader counts (the lines
beginning with console.log('[With loader] ...')), leaving only the assertions;
this keeps test output clean while preserving the existing loader/render
assertions and test behavior.
Contributor
|
please remove the console logs |
When a route uses an inline arrow function for its component (common in file-based routing), React Refresh cannot register the component for HMR updates. The router's HMR handler calls router.invalidate() to trigger updates, but the MatchInner component's useRouterState selector did not include the 'invalid' field, so React didn't detect the state change and didn't re-render. This fix adds 'invalid: match.invalid' to the selector, ensuring React detects the state change when invalidate() is called and re-renders the component tree, picking up the new component reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
longfellowone
force-pushed
the
fix/hmr-inline-arrow-components
branch
from
485fc06 to
fee6467
Compare
Contributor
|
about the async loader test: component does not consume the loader data, so it shouldn't rerender because of loader finishing. please modify the test to consume the loader data and also let the loader return new data the second time. then check if new component has new loader data |
Per reviewer feedback, the async loader test now: - Consumes loader data via useLoaderData() - Loader returns different data each call (loaded-1 → loaded-2) - Verifies new component receives new loader data - Expects 2 renders (1 for invalidation, 1 for new data) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6195
Fixes HMR (Hot Module Replacement) not updating the UI when route components are defined as inline arrow functions. This regression appeared around v1.128.3 (worked in v1.128.0).
Problem
When a route uses an inline arrow function for its
componentoption (common in file-based routing), React Refresh cannot register the component for HMR updates. The router's HMR handler compensates by callingrouter.invalidate()to trigger updates, but theMatchInnercomponent'suseRouterStateselector did not include theinvalidfield. This meant React didn't detect any state change and didn't re-render, so the UI stayed on the old component even though HMR was triggered.Named function components still work because React Refresh can register them directly, bypassing this issue.
Solution
Add
invalid: match.invalidto theMatchInnercomponent'suseRouterStateselector. This ensures React detects the state change wheninvalidate()is called and re-renders the component tree, picking up the new component reference.Changes
packages/react-router/src/Match.tsx: Addinvalidto the selector (1 line)packages/react-router/tests/router.test.tsx: Add regression test that:invalidate()Test plan
examples/react/basic-file-basedwith inline arrow function🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
invalidproperty for improved routing state visibility.Tests
✏️ Tip: You can customize this high-level summary in your review settings.