fix: shared virtual module for code-split routes#6659
Conversation
When a route file has module-level bindings (variables, functions, classes) referenced by **both** split and non-split route properties, the bindings are duplicated — initialized independently in the reference file and in each virtual split file. This breaks singletons, causes double side effects, and wastes bundle size.
```tsx
const collection = { name: 'todos', preload: async () => {} }
export const Route = createFileRoute('/todos')({
loader: async () => {
await collection.preload()
}, // stays in reference
component: () => <div>{collection.name}</div>, // split to virtual
})
```
In this example, `collection` is initialized in **both** the reference file (because `loader` references it) and the `?tsr-split=component` virtual file (because the component references it). Two separate instances exist at runtime.
To solve this issue, this PR introduces a third virtual module `route.tsx?tsr-shared=1` that contains shared bindings. Both the reference and virtual files import from it.
When `sharedBindings` is empty (the common case — component-only helpers, loader-only code), no shared module exists and the system works exactly as today.
The reference transform runs first and will emit a static import of `?tsr-shared=1` **only when needed**. Since split chunks are only discovered via the reference file's emitted `import('...?tsr-split=...')`, the bundler will always process/evaluate the reference module first. Result: the shared module is evaluated once and then reused by any later-loaded split chunk.
📝 WalkthroughWalkthroughAdds shared-bindings support to the router-plugin code-splitter: static analysis to detect module-level bindings that must be pulled into a shared virtual module, compilation of those shared modules, integration into the existing split/virtual/reference compilation pipeline, E2E test + route for a shared singleton, and extensive snapshot/test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant RP as Router Plugin
participant CS as Code Splitter (compilers)
participant Shared as Shared Virtual Module
participant Ref as Reference Route
participant Virt as Virtual (split) Route
RP->>CS: computeSharedBindings(code, codeSplitGroupings)
CS->>CS: buildDeclarationMap(ast)\nbuildDependencyGraph(...)\ncollectIdentifiersFromNode(...)
CS-->>RP: Set<sharedBindings>
RP->>CS: compileCodeSplitSharedRoute(filename, sharedBindings)
CS->>Shared: emit shared virtual module\n(imports/exports only shared bindings)
CS-->>RP: GeneratorResult(shared module)
RP->>CS: compileCodeSplitReferenceRoute(code, sharedBindings)
CS->>Ref: inject imports from shared virtual module\nregister imported specifiers for DCE
CS-->>RP: GeneratorResult(reference chunk)
RP->>CS: compileCodeSplitVirtualRoute(splitTargets, sharedBindings)
CS->>Virt: import + re-export shared bindings\napply DCE cleanup
CS-->>RP: GeneratorResult(virtual/split chunk)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
|
View your CI Pipeline Execution ↗ for commit a0323f2
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/router-plugin/src/core/code-splitter/compilers.ts`:
- Around line 1395-1406: expandTransitively can pull the singleton symbol Route
into keepBindings (initialized from opts.sharedBindings), violating the
invariant that Route must never be extracted; after building depGraph (and
before calling expandTransitively) detect any binding that transitively depends
on the symbol "Route" (e.g., walk depGraph from each candidate in
opts.sharedBindings or compute a reverse-dependency reachability to "Route") and
either remove those bindings from keepBindings or throw a clear error indicating
the offending binding(s). Update references to
expandTransitively/keepBindings/depGraph/buildDependencyGraph/opts.sharedBindings
accordingly so the shared-set expansion will not include any binding that
depends on "Route".
- Around line 1358-1371: The current filter in the AST transformer removes
top-level ExpressionStatements even when they are directive prologues (e.g.,
'use client' / 'use server'); update the filter inside the ast.program.body
reassignment so that any ExpressionStatement with a .directive property (i.e., a
directive prologue) is always preserved before applying the existing
local-binding check; locate the filter that uses
collectLocalBindingsFromStatement and collectIdentifiersFromNode and add an
explicit early-return true for statements where t.isExpressionStatement(stmt) &&
stmt.directive to retain runtime semantics.
🧹 Nitpick comments (2)
packages/router-plugin/tests/code-splitter/test-files/react/shared-function.tsx (1)
3-6: Consider adding type parameters for stricter typing.The
Maplacks type parameters andgetCachedreturns an implicitany. For TypeScript strict mode compliance, explicit types would be preferable.💡 Suggested improvement
-const cache = new Map() -function getCached(key: string) { +const cache = new Map<string, unknown>() +function getCached(key: string): unknown { return cache.get(key) }That said, this is a test fixture focused on demonstrating shared bindings for code-splitting, so the simpler form may be intentional to keep the test focused.
e2e/react-router/basic-file-based-code-splitting/src/routes/shared-singleton.tsx (1)
5-14: Avoidanyfor the shared singleton global.Casting
globalThistoanyweakens strict typing; a typed global keeps the test strict-mode friendly.As per coding guidelines, "**/*.{ts,tsx}**: Use TypeScript strict mode with extensive type safety for all code".♻️ Suggested typing improvement
import * as React from 'react' +declare global { + var __tsrSharedSingleton: { initCount: number } | undefined +} + // All shared state lives in declarations — no bare expression statements. // The singleton tracks how many times this module scope executes. const singleton = (() => { - const g = globalThis as any - g.__tsrSharedSingleton ??= { initCount: 0 } - g.__tsrSharedSingleton.initCount++ - return { initCountAtCreate: g.__tsrSharedSingleton.initCount as number } + globalThis.__tsrSharedSingleton ??= { initCount: 0 } + globalThis.__tsrSharedSingleton.initCount++ + return { initCountAtCreate: globalThis.__tsrSharedSingleton.initCount } })() function getInitCount() { - return (globalThis as any).__tsrSharedSingleton?.initCount as number + return globalThis.__tsrSharedSingleton?.initCount ?? 0 }
…dBindings Skip the expensive babel.traverse for route files that have no local module-level bindings (aside from Route). Most route files only contain imports + the Route export, so this early bailout avoids the traversal in the common case.
SeanCassiere
left a comment
There was a problem hiding this comment.
The tests make sense! which is always a good thing 🙌🏼
This is probably something that should've been done during the implementation of the code-splitting groups, but I think its time we probably have some internal documentation for devs touching the code-splitter.
Mostly since there's a decent bit of complexity whenever the code-splitter gets touched. Plus, changes here often times mean that 50+ snapshots see changes making it easier for unintended things slipping through. A mermaid diagram or something would help I'd think, at-least in the understanding of the conditions and actions of running the code-splitter.
Long term, we likely break apart the tests into a very different structure, but we're probably not there just yet.
… shared bindings Add 76 new tests across three layers for the shared virtual module code-splitting logic introduced in TanStack#6659: Layer 1 - Algebraic property tests on helper functions: - expandTransitively: idempotence, monotonicity, cycle handling - buildDependencyGraph: subset invariants - removeBindingsDependingOnRoute: direct/transitive removal - expandDestructuredDeclarations: idempotence, sibling expansion - collectLocalBindingsFromStatement: all declaration types Layer 2 - Invariant tests on computeSharedBindings: - Route is never in the shared set - Results are always real local bindings (never imports) - Destructured siblings cohesion - Transitive dependency inclusion - Route-dependent binding exclusion - Determinism across repeated calls Layer 3 - Small-scope exhaustive tests: - Single binding × 3 declaration kinds × 4 property patterns × 2 groupings - Two bindings × 4 cross-group reference patterns × 2 groupings - Transitive dependency chains (linear, 3-deep, diamond) - Meta-invariant conformance across all shared-* fixture files Testing approach inspired by: https://gist.github.com/KyleAMathews/64bcea53a57f600cc3b73bc366d9ac4d Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ngs (#6662) * test: add invariant, property, and exhaustive tests for code-splitter shared bindings Add 76 new tests across three layers for the shared virtual module code-splitting logic introduced in #6659: Layer 1 - Algebraic property tests on helper functions: - expandTransitively: idempotence, monotonicity, cycle handling - buildDependencyGraph: subset invariants - removeBindingsDependingOnRoute: direct/transitive removal - expandDestructuredDeclarations: idempotence, sibling expansion - collectLocalBindingsFromStatement: all declaration types Layer 2 - Invariant tests on computeSharedBindings: - Route is never in the shared set - Results are always real local bindings (never imports) - Destructured siblings cohesion - Transitive dependency inclusion - Route-dependent binding exclusion - Determinism across repeated calls Layer 3 - Small-scope exhaustive tests: - Single binding × 3 declaration kinds × 4 property patterns × 2 groupings - Two bindings × 4 cross-group reference patterns × 2 groupings - Transitive dependency chains (linear, 3-deep, diamond) - Meta-invariant conformance across all shared-* fixture files Testing approach inspired by: https://gist.github.com/KyleAMathews/64bcea53a57f600cc3b73bc366d9ac4d Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: apply automated fixes --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
* fix: shared virtual module for code-split routes
When a route file has module-level bindings (variables, functions, classes) referenced by **both** split and non-split route properties, the bindings are duplicated — initialized independently in the reference file and in each virtual split file. This breaks singletons, causes double side effects, and wastes bundle size.
```tsx
const collection = { name: 'todos', preload: async () => {} }
export const Route = createFileRoute('/todos')({
loader: async () => {
await collection.preload()
}, // stays in reference
component: () => <div>{collection.name}</div>, // split to virtual
})
```
In this example, `collection` is initialized in **both** the reference file (because `loader` references it) and the `?tsr-split=component` virtual file (because the component references it). Two separate instances exist at runtime.
To solve this issue, this PR introduces a third virtual module `route.tsx?tsr-shared=1` that contains shared bindings. Both the reference and virtual files import from it.
When `sharedBindings` is empty (the common case — component-only helpers, loader-only code), no shared module exists and the system works exactly as today.
The reference transform runs first and will emit a static import of `?tsr-shared=1` **only when needed**. Since split chunks are only discovered via the reference file's emitted `import('...?tsr-split=...')`, the bundler will always process/evaluate the reference module first. Result: the shared module is evaluated once and then reused by any later-loaded split chunk.
…ngs (#6662) * test: add invariant, property, and exhaustive tests for code-splitter shared bindings Add 76 new tests across three layers for the shared virtual module code-splitting logic introduced in #6659: Layer 1 - Algebraic property tests on helper functions: - expandTransitively: idempotence, monotonicity, cycle handling - buildDependencyGraph: subset invariants - removeBindingsDependingOnRoute: direct/transitive removal - expandDestructuredDeclarations: idempotence, sibling expansion - collectLocalBindingsFromStatement: all declaration types Layer 2 - Invariant tests on computeSharedBindings: - Route is never in the shared set - Results are always real local bindings (never imports) - Destructured siblings cohesion - Transitive dependency inclusion - Route-dependent binding exclusion - Determinism across repeated calls Layer 3 - Small-scope exhaustive tests: - Single binding × 3 declaration kinds × 4 property patterns × 2 groupings - Two bindings × 4 cross-group reference patterns × 2 groupings - Transitive dependency chains (linear, 3-deep, diamond) - Meta-invariant conformance across all shared-* fixture files Testing approach inspired by: https://gist.github.com/KyleAMathews/64bcea53a57f600cc3b73bc366d9ac4d Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: apply automated fixes --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
When a route file has module-level bindings (variables, functions, classes) referenced by both split and non-split route properties, the bindings are duplicated — initialized independently in the reference file and in each virtual split file. This breaks singletons, causes double side effects, and wastes bundle size.
In this example,
collectionis initialized in both the reference file (becauseloaderreferences it) and the?tsr-split=componentvirtual file (because the component references it). Two separate instances exist at runtime.To solve this issue, this PR introduces a third virtual module
route.tsx?tsr-shared=1that contains shared bindings. Both the reference and virtual files import from it.When
sharedBindingsis empty (the common case — component-only helpers, loader-only code), no shared module exists and the system works exactly as today.The reference transform runs first and will emit a static import of
?tsr-shared=1only when needed. Since split chunks are only discovered via the reference file's emittedimport('...?tsr-split=...'), the bundler will always process/evaluate the reference module first. Result: the shared module is evaluated once and then reused by any later-loaded split chunk.Summary by CodeRabbit
New Features
Tests