Skip to content

Commit cf66184

Browse files
KyleAMathewsclaude
andcommitted
fix(router-plugin): fix critical code splitting bugs identified by reviewer
Addresses three critical runtime bugs found in PR review: 1. **Bare specifier imports** - Dynamic imports and static imports were using bare specifiers (e.g., "file.tsx") instead of relative paths ("./file.tsx"), causing bundler resolution failures. 2. **Importing non-exported variables** - Virtual split files were importing variables that weren't exported in the reference file, resulting in undefined at runtime. 3. **Missing metadata flow** - Virtual compiler had no way to know which variables were promoted to exports in the reference file. **Solutions implemented:** - Updated `addSplitSearchParamFromFilename` and `removeSplitSearchParamFromFilename` to ensure all import specifiers use relative paths with "./" prefix - Added `sharedExports` metadata flow from reference compilation to virtual compilation via plugin-layer cache - Virtual compiler now only imports variables confirmed to be in the shared exports set, preventing undefined imports - Updated tests to simulate the plugin's caching behavior **Edge cases tested:** - Multiple shared variables - Already exported variables - Multiple declarations in same statement - Call expression initializers - Variable dependencies - Objects with methods - Nested function usage All 288 tests passing. Users no longer need to manually export variables - the code splitter automatically handles sharing between split and non-split parts. Reviewer feedback: #5767 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent a11f578 commit cf66184

File tree

266 files changed

+1902
-221
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

266 files changed

+1902
-221
lines changed

packages/router-plugin/src/core/code-splitter/compilers.ts

Lines changed: 47 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,24 @@ function addSplitSearchParamToFilename(
8181
) {
8282
const [bareFilename] = filename.split('?')
8383

84+
// Ensure relative import specifier for proper bundler resolution
85+
const relativeFilename = bareFilename!.startsWith('./') || bareFilename!.startsWith('../')
86+
? bareFilename!
87+
: `./${bareFilename!}`
88+
8489
const params = new URLSearchParams()
8590
params.append(tsrSplit, createIdentifier(grouping))
8691

87-
const result = `${bareFilename}?${params.toString()}`
92+
const result = `${relativeFilename}?${params.toString()}`
8893
return result
8994
}
9095

9196
function removeSplitSearchParamFromFilename(filename: string) {
9297
const [bareFilename] = filename.split('?')
93-
return bareFilename!
98+
// Ensure relative import specifier for proper bundler resolution
99+
return bareFilename!.startsWith('./') || bareFilename!.startsWith('../')
100+
? bareFilename!
101+
: `./${bareFilename!}`
94102
}
95103

96104
const splittableCreateRouteFns = ['createFileRoute']
@@ -111,6 +119,7 @@ export function compileCodeSplitReferenceRoute(
111119
filename: string
112120
id: string
113121
addHmr?: boolean
122+
sharedExports?: Set<string>
114123
},
115124
): GeneratorResult | null {
116125
const ast = parseAst(opts)
@@ -536,6 +545,8 @@ export function compileCodeSplitReferenceRoute(
536545
const exportDecl = t.exportNamedDeclaration(varDecl.node, [])
537546
varDecl.replaceWith(exportDecl)
538547
knownExportedIdents.add(identName)
548+
// Track in sharedExports for virtual compiler to use
549+
opts.sharedExports?.add(identName)
539550
}
540551
}
541552
})
@@ -580,6 +591,7 @@ export function compileCodeSplitVirtualRoute(
580591
opts: ParseAstOptions & {
581592
splitTargets: Array<SplitRouteIdentNodes>
582593
filename: string
594+
sharedExports?: Set<string>
583595
},
584596
): GeneratorResult {
585597
const ast = parseAst(opts)
@@ -880,86 +892,52 @@ export function compileCodeSplitVirtualRoute(
880892
},
881893
})
882894

883-
// Convert non-exported module-level variables that are shared with the reference file
884-
// to imports. These are variables used by both split and non-split parts.
885-
// They will be exported in the reference file, so we need to import them here.
886-
const importedSharedIdents = new Set<string>()
887-
const sharedVariablesToImport: Array<string> = []
888-
889-
programPath.traverse({
890-
VariableDeclaration(varDeclPath) {
891-
// Only process top-level const/let declarations
892-
if (!varDeclPath.parentPath.isProgram()) return
893-
if (varDeclPath.node.kind !== 'const' && varDeclPath.node.kind !== 'let') return
894-
895-
varDeclPath.node.declarations.forEach((declarator) => {
896-
if (!t.isIdentifier(declarator.id)) return
895+
// Import shared module-level variables that were exported in the reference file
896+
// Only process variables confirmed to be in the sharedExports set
897+
if (opts.sharedExports && opts.sharedExports.size > 0) {
898+
const variablesToImport: Array<string> = []
897899

898-
const varName = declarator.id.name
899-
900-
// Skip if the init is a function/arrow function - those are unlikely to be shared objects
901-
// We only want to import object literals and similar data structures
902-
if (
903-
t.isArrowFunctionExpression(declarator.init) ||
904-
t.isFunctionExpression(declarator.init)
905-
) {
906-
return
907-
}
908-
909-
// Check if this variable is used by the split node
910-
// If it is, it might be shared with non-split parts and should be imported
911-
const isUsedBySplitNode = intendedSplitNodes.values().next().value &&
912-
Object.values(trackedNodesToSplitByType).some(tracked => {
913-
if (!tracked?.node) return false
914-
let isUsed = false
915-
babel.traverse(tracked.node, {
916-
Identifier(idPath) {
917-
if (idPath.isReferencedIdentifier() && idPath.node.name === varName) {
918-
isUsed = true
919-
}
920-
}
921-
}, programPath.scope)
922-
return isUsed
923-
})
924-
925-
if (isUsedBySplitNode) {
926-
sharedVariablesToImport.push(varName)
927-
}
928-
})
929-
},
930-
})
931-
932-
// Remove shared variable declarations and add imports
933-
if (sharedVariablesToImport.length > 0) {
934900
programPath.traverse({
935901
VariableDeclaration(varDeclPath) {
902+
// Only process top-level const/let declarations
936903
if (!varDeclPath.parentPath.isProgram()) return
904+
if (varDeclPath.node.kind !== 'const' && varDeclPath.node.kind !== 'let') return
905+
906+
varDeclPath.node.declarations.forEach((declarator) => {
907+
if (!t.isIdentifier(declarator.id)) return
937908

938-
const declaratorsToKeep = varDeclPath.node.declarations.filter((declarator) => {
939-
if (!t.isIdentifier(declarator.id)) return true
940909
const varName = declarator.id.name
941910

942-
if (sharedVariablesToImport.includes(varName)) {
943-
importedSharedIdents.add(varName)
944-
return false // Remove this declarator
911+
// Only import if this variable is in the confirmed shared exports set
912+
if (opts.sharedExports!.has(varName)) {
913+
variablesToImport.push(varName)
945914
}
946-
return true
947915
})
948-
949-
if (declaratorsToKeep.length === 0) {
950-
// Remove the entire variable declaration
951-
varDeclPath.remove()
952-
} else if (declaratorsToKeep.length < varDeclPath.node.declarations.length) {
953-
// Update with remaining declarators
954-
varDeclPath.node.declarations = declaratorsToKeep
955-
}
956916
},
957917
})
958918

959-
// Add import statement for shared variables
960-
if (importedSharedIdents.size > 0) {
919+
// Remove shared variable declarations and add imports
920+
if (variablesToImport.length > 0) {
921+
programPath.traverse({
922+
VariableDeclaration(varDeclPath) {
923+
if (!varDeclPath.parentPath.isProgram()) return
924+
925+
const declaratorsToKeep = varDeclPath.node.declarations.filter((declarator) => {
926+
if (!t.isIdentifier(declarator.id)) return true
927+
return !variablesToImport.includes(declarator.id.name)
928+
})
929+
930+
if (declaratorsToKeep.length === 0) {
931+
varDeclPath.remove()
932+
} else if (declaratorsToKeep.length < varDeclPath.node.declarations.length) {
933+
varDeclPath.node.declarations = declaratorsToKeep
934+
}
935+
},
936+
})
937+
938+
// Add import statement for shared variables
961939
const importDecl = t.importDeclaration(
962-
Array.from(importedSharedIdents).map((name) =>
940+
variablesToImport.map((name) =>
963941
t.importSpecifier(t.identifier(name), t.identifier(name)),
964942
),
965943
t.stringLiteral(removeSplitSearchParamFromFilename(opts.filename)),

packages/router-plugin/src/core/router-code-splitter-plugin.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ export const unpluginRouterCodeSplitterFactory: UnpluginFactory<
6666
let ROOT: string = process.cwd()
6767
let userConfig: Config
6868

69+
// Cache of shared module-level variables that were exported in reference files
70+
// Key: base filename (without query params), Value: Set of exported identifier names
71+
const sharedExportsCache = new Map<string, Set<string>>()
72+
6973
function initUserConfig() {
7074
if (typeof options === 'function') {
7175
userConfig = options()
@@ -125,6 +129,7 @@ export const unpluginRouterCodeSplitterFactory: UnpluginFactory<
125129
const splitGroupings: CodeSplitGroupings =
126130
fromCode.groupings || pluginSplitBehavior || getGlobalCodeSplitGroupings()
127131

132+
const sharedExports = new Set<string>()
128133
const compiledReferenceRoute = compileCodeSplitReferenceRoute({
129134
code,
130135
codeSplitGroupings: splitGroupings,
@@ -136,8 +141,15 @@ export const unpluginRouterCodeSplitterFactory: UnpluginFactory<
136141
: undefined,
137142
addHmr:
138143
(userConfig.codeSplittingOptions?.addHmr ?? true) && !isProduction,
144+
sharedExports,
139145
})
140146

147+
// Cache shared exports for virtual route compilation
148+
if (sharedExports.size > 0) {
149+
const baseFilename = id.split('?')[0]!
150+
sharedExportsCache.set(baseFilename, sharedExports)
151+
}
152+
141153
if (compiledReferenceRoute === null) {
142154
if (debug) {
143155
console.info(
@@ -176,10 +188,15 @@ export const unpluginRouterCodeSplitterFactory: UnpluginFactory<
176188
splitRouteIdentNodes.includes(p as any),
177189
) as Array<SplitRouteIdentNodes>
178190

191+
// Get shared exports from cache for this route
192+
const baseFilename = id.split('?')[0]!
193+
const sharedExports = sharedExportsCache.get(baseFilename)
194+
179195
const result = compileCodeSplitVirtualRoute({
180196
code,
181197
filename: id,
182198
splitTargets: grouping,
199+
sharedExports,
183200
})
184201

185202
if (debug) {

packages/router-plugin/tests/code-splitter.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,21 +52,31 @@ describe('code-splitter works', () => {
5252
const dirs = getFrameworkDir(framework)
5353
const filenames = await readdir(dirs.files)
5454

55+
// Cache for shared exports per filename
56+
const sharedExportsCache = new Map<string, Set<string>>()
57+
5558
it.each(filenames)(
5659
`should compile "reference" for "%s"`,
5760
async (filename) => {
5861
const file = await readFile(path.join(dirs.files, filename))
5962
const code = file.toString()
6063

64+
const sharedExports = new Set<string>()
6165
const compileResult = compileCodeSplitReferenceRoute({
6266
code,
6367
filename,
6468
id: filename,
6569
addHmr: false,
6670
codeSplitGroupings: grouping,
6771
targetFramework: framework,
72+
sharedExports,
6873
})
6974

75+
// Cache shared exports for virtual compilation
76+
if (sharedExports.size > 0) {
77+
sharedExportsCache.set(filename, sharedExports)
78+
}
79+
7080
await expect(compileResult?.code || code).toMatchFileSnapshot(
7181
path.join(dirs.snapshots, groupName, filename),
7282
)
@@ -79,13 +89,17 @@ describe('code-splitter works', () => {
7989
const file = await readFile(path.join(dirs.files, filename))
8090
const code = file.toString()
8191

92+
// Get shared exports from cache
93+
const sharedExports = sharedExportsCache.get(filename)
94+
8295
for (const targets of grouping) {
8396
const ident = createIdentifier(targets)
8497

8598
const splitResult = compileCodeSplitVirtualRoute({
8699
code,
87100
filename: `${filename}?${ident}`,
88101
splitTargets: targets,
102+
sharedExports,
89103
})
90104

91105
const snapshotFilename = path.join(

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/arrow-function.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('arrow-function.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./arrow-function.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import * as React from 'react';
44
import { createFileRoute } from '@tanstack/react-router';

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/arrow-function@component.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as React from 'react';
22
import { Link, Outlet } from '@tanstack/react-router';
3-
import { Route } from "arrow-function.tsx";
3+
import { Route } from "./arrow-function.tsx";
44
const PostsComponent = () => {
55
const posts = Route.useLoaderData();
66
return <div className="p-2 flex gap-2">

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/boolean-null-literals.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('boolean-null-literals.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./boolean-null-literals.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import { createFileRoute } from '@tanstack/router';
44

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/chinese.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('chinese.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./chinese.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import * as React from 'react';
44
import { createFileRoute } from '@tanstack/react-router';

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/circular-reference-arrow-function.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('circular-reference-arrow-function.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./circular-reference-arrow-function.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import { createFileRoute } from '@tanstack/react-router';
44
export const Route = createFileRoute('/')({

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/circular-reference-function.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('circular-reference-function.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./circular-reference-function.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import { createFileRoute } from '@tanstack/react-router';
44
export const Route = createFileRoute('/')({

packages/router-plugin/tests/code-splitter/snapshots/react/1-default/conditional-properties.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const $$splitComponentImporter = () => import('conditional-properties.tsx?tsr-split=component');
1+
const $$splitComponentImporter = () => import('./conditional-properties.tsx?tsr-split=component');
22
import { lazyRouteComponent } from '@tanstack/react-router';
33
import { createFileRoute } from '@tanstack/react-router';
44
import { isEnabled } from '@features/feature-flags';

0 commit comments

Comments
 (0)