fix: hoist inline component definitions for proper React HMR#6919
Merged
schiller-manuel merged 2 commits intomainfrom Mar 13, 2026
Merged
fix: hoist inline component definitions for proper React HMR#6919schiller-manuel merged 2 commits intomainfrom
schiller-manuel merged 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: eb199dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📝 WalkthroughWalkthroughAdds a compiler-plugin extension point to the code-splitting pipeline with a React-specific HMR plugin that hoists inline route components; removes Changes
Sequence Diagram(s)sequenceDiagram
participant RouterHMR as Router HMR Plugin
participant CodeSplitter as Code Splitter / Compiler
participant Plugin as ReferenceRouteCompilerPlugin
participant AST as Babel AST
alt target == "react" and addHmr == true
RouterHMR->>CodeSplitter: compileCodeSplitReferenceRoute(opts + compilerPlugins)
CodeSplitter->>CodeSplitter: analyze route (splittable?)
alt route unsplittable
CodeSplitter->>Plugin: onUnsplittableRoute(context)
Plugin->>AST: identify inline component properties
AST->>AST: hoist functions to top-level consts (unique ids)
AST->>CodeSplitter: rewrite route props to use hoisted ids
Plugin-->>CodeSplitter: result { modified: true }
end
CodeSplitter-->>RouterHMR: compiled code
RouterHMR->>RouterHMR: return compiled output (short-circuit AST insertion)
else non-React or compilation failed
RouterHMR->>AST: parse/transform AST and push HMR statements
AST-->>RouterHMR: generate code
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
View your CI Pipeline Execution ↗ for commit eb199dd
☁️ Nx Cloud last updated this comment at |
Contributor
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/router-plugin/src/core/code-splitter/plugins.ts (1)
16-23: Tighten the new plugin context types before this API spreads.
insertionPathandcreateRouteFnare both very broad here, so plugin implementations lose useful narrowing right at the boundary and will end up casting. If the compiler only passes a small node union and a fixed set of route-factory names, model that explicitly in this context type now.As per coding guidelines "Use TypeScript strict mode with extensive type safety".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/src/core/code-splitter/plugins.ts` around lines 16 - 23, The ReferenceRouteCompilerPluginContext type is too broad: narrow insertionPath and createRouteFn so downstream plugins get precise types; change insertionPath from babel.NodePath (any) to a NodePath union that matches only the actual insertion targets (e.g., babel.NodePath<t.VariableDeclarator | t.ExpressionStatement | t.Program> or whatever small set the compiler uses) and change createRouteFn from string to a literal union of allowed factory names (e.g., 'createRoute' | 'createReferenceRoute' or the exact names your compiler emits); update the type definition for ReferenceRouteCompilerPluginContext accordingly so plugins can rely on narrowed node kinds and known factory names without casting.packages/router-plugin/src/core/router-hmr-plugin.ts (1)
46-67: Add one transform-level test for this React fast path.The new behavior is wired here, but the added coverage in
packages/router-plugin/tests/add-hmr.test.tsexercisescompileCodeSplitReferenceRoutedirectly rather thanunpluginRouterHmrFactory.transform.handler. A small integration test around this branch would protect the actual regression point from future drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/src/core/router-hmr-plugin.ts` around lines 46 - 67, Add a transform-level integration test that exercises the React fast path in unpluginRouterHmrFactory.transform.handler (the branch that checks userConfig.target === 'react' and returns the result of compileCodeSplitReferenceRoute). The test should invoke unpluginRouterHmrFactory.transform.handler with a mock/fixture module id and source code that would trigger compileCodeSplitReferenceRoute, set userConfig.target to 'react', and assert that the handler returns the compiled result (or contains expected compiled.code/HMR markers) rather than calling compileCodeSplitReferenceRoute directly; target the same behavior currently guarded in router-hmr-plugin.ts so the actual transform pipeline is covered end-to-end. Ensure the test filename/location mirrors other transform tests (e.g., tests/add-hmr.test.ts style) and mocks any required plugin context/environment used by transform.handler.packages/router-plugin/src/core/code-splitter/compilers.ts (1)
640-650: Deriveoptsfrom the shared options type instead of asserting it.
compileCodeSplitReferenceRoutenow keeps a second copy of the reference-route options shape, and Line 731 has to cast back toCompileCodeSplitReferenceRouteOptionsto satisfy the plugin context. Reusing the shared type in this function signature would let strict mode catch drift instead of masking it.♻️ Suggested change
+type CompileCodeSplitReferenceRouteWithPluginsOptions = + ParseAstOptions & + CompileCodeSplitReferenceRouteOptions & { + compilerPlugins?: Array<ReferenceRouteCompilerPlugin> + } + export function compileCodeSplitReferenceRoute( - opts: ParseAstOptions & { - codeSplitGroupings: CodeSplitGroupings - deleteNodes?: Set<DeletableNodes> - targetFramework: Config['target'] - filename: string - id: string - addHmr?: boolean - sharedBindings?: Set<string> - compilerPlugins?: Array<ReferenceRouteCompilerPlugin> - }, + opts: CompileCodeSplitReferenceRouteWithPluginsOptions, ): GeneratorResult | null { @@ - opts: opts as CompileCodeSplitReferenceRouteOptions, + opts,As per coding guidelines,
**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.Also applies to: 731-731
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router-plugin/src/core/code-splitter/compilers.ts` around lines 640 - 650, The function compileCodeSplitReferenceRoute is duplicating the reference-route options shape; change its opts parameter to use the existing shared reference-route options type (the shared options type used elsewhere) instead of redefining the shape inline, and remove the runtime/type cast to CompileCodeSplitReferenceRouteOptions at the plugin context usage (the cast around the plugin invocation at the current plugin context site). In practice: import/reference the shared options type, replace the inline opts type in compileCodeSplitReferenceRoute with that shared type, and update any code that currently casts opts back to CompileCodeSplitReferenceRouteOptions so the compiler no longer requires the cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/router-plugin/src/core/code-splitter/compilers.ts`:
- Around line 640-650: The function compileCodeSplitReferenceRoute is
duplicating the reference-route options shape; change its opts parameter to use
the existing shared reference-route options type (the shared options type used
elsewhere) instead of redefining the shape inline, and remove the runtime/type
cast to CompileCodeSplitReferenceRouteOptions at the plugin context usage (the
cast around the plugin invocation at the current plugin context site). In
practice: import/reference the shared options type, replace the inline opts type
in compileCodeSplitReferenceRoute with that shared type, and update any code
that currently casts opts back to CompileCodeSplitReferenceRouteOptions so the
compiler no longer requires the cast.
In `@packages/router-plugin/src/core/code-splitter/plugins.ts`:
- Around line 16-23: The ReferenceRouteCompilerPluginContext type is too broad:
narrow insertionPath and createRouteFn so downstream plugins get precise types;
change insertionPath from babel.NodePath (any) to a NodePath union that matches
only the actual insertion targets (e.g., babel.NodePath<t.VariableDeclarator |
t.ExpressionStatement | t.Program> or whatever small set the compiler uses) and
change createRouteFn from string to a literal union of allowed factory names
(e.g., 'createRoute' | 'createReferenceRoute' or the exact names your compiler
emits); update the type definition for ReferenceRouteCompilerPluginContext
accordingly so plugins can rely on narrowed node kinds and known factory names
without casting.
In `@packages/router-plugin/src/core/router-hmr-plugin.ts`:
- Around line 46-67: Add a transform-level integration test that exercises the
React fast path in unpluginRouterHmrFactory.transform.handler (the branch that
checks userConfig.target === 'react' and returns the result of
compileCodeSplitReferenceRoute). The test should invoke
unpluginRouterHmrFactory.transform.handler with a mock/fixture module id and
source code that would trigger compileCodeSplitReferenceRoute, set
userConfig.target to 'react', and assert that the handler returns the compiled
result (or contains expected compiled.code/HMR markers) rather than calling
compileCodeSplitReferenceRoute directly; target the same behavior currently
guarded in router-hmr-plugin.ts so the actual transform pipeline is covered
end-to-end. Ensure the test filename/location mirrors other transform tests
(e.g., tests/add-hmr.test.ts style) and mocks any required plugin
context/environment used by transform.handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 792b6a3d-38b0-4907-843c-2e235b3d354c
📒 Files selected for processing (14)
packages/react-router/src/Match.tsxpackages/react-router/tests/router.test.tsxpackages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/src/core/code-splitter/plugins.tspackages/router-plugin/src/core/code-splitter/plugins/framework-plugins.tspackages/router-plugin/src/core/code-splitter/plugins/react-refresh-route-components.tspackages/router-plugin/src/core/router-code-splitter-plugin.tspackages/router-plugin/src/core/router-hmr-plugin.tspackages/router-plugin/src/core/utils.tspackages/router-plugin/tests/add-hmr.test.tspackages/router-plugin/tests/add-hmr/snapshots/react/createRootRoute-inline-component@false.tsxpackages/router-plugin/tests/add-hmr/snapshots/react/createRootRoute-inline-component@true.tsxpackages/router-plugin/tests/add-hmr/test-files/react/createRootRoute-inline-component.tsxpackages/router-plugin/tests/utils.test.ts
💤 Files with no reviewable changes (2)
- packages/react-router/src/Match.tsx
- packages/react-router/tests/router.test.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/soft-pianos-lie.md:
- Line 6: Update the changeset title string "fix: hoist inline component
definitions for proper React HMR#6919" to add a space before the PR reference so
it reads "...React HMR `#6919`", and append a short issue reference such as "
(fixes `#6339`)" or " — fixes `#6339`" to indicate this patch resolves the hard page
reloads issue; locate and edit the same line in the .changeset entry that
contains the current title text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2782d24-2459-48f4-9605-592bf67b3853
📒 Files selected for processing (1)
.changeset/soft-pianos-lie.md
| '@tanstack/react-router': patch | ||
| --- | ||
|
|
||
| fix: hoist inline component definitions for proper React HMR#6919 |
Contributor
There was a problem hiding this comment.
Add space before PR reference and consider mentioning the fixed issue.
The description is missing a space between "HMR" and "#6919", which reduces readability. Additionally, consider referencing issue #6339 to help users understand this fix resolves hard page reloads during development.
📝 Suggested improvement
-fix: hoist inline component definitions for proper React HMR#6919
+fix: hoist inline component definitions for proper React HMR (`#6919`, fixes `#6339`)Or alternatively, make it more user-focused:
-fix: hoist inline component definitions for proper React HMR#6919
+fix: prevent hard page reloads on save by hoisting inline component definitions (`#6919`, fixes `#6339`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/soft-pianos-lie.md at line 6, Update the changeset title string
"fix: hoist inline component definitions for proper React HMR#6919" to add a
space before the PR reference so it reads "...React HMR `#6919`", and append a
short issue reference such as " (fixes `#6339`)" or " — fixes `#6339`" to indicate
this patch resolves the hard page reloads issue; locate and edit the same line
in the .changeset entry that contains the current title text.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
this reverts #6197 and implements a proper fix
fixes #6339
Summary by CodeRabbit
Breaking Changes
invalidproperty from match payloads.New Features
Refactor
Tests