fix(router-plugin): show warning for non-splittable items #5145
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
WalkthroughIntroduces export-tracking in the router plugin’s code-split compiler to detect non-splittable exports and inject a runtime console warning in reference route files (non-production). Removes prior warnings from virtual submodules. Adds documentation describing splitting rules. Updates test snapshots to reflect new warning placement and minor import/const tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Build as Router Plugin (Build)
participant FS as Route File (reference)
participant Virt as Virtual Submodules
participant App as App Runtime
Dev->>Build: Build project
Build->>FS: Analyze exports in route file
alt Exported identifiers found
Build->>Build: Track exported idents (knownExportedIdents)
Build->>FS: Inject runtime console.warn (non-production)
Build->>Build: Mark shouldSplit=false for those idents
else No conflicting exports
Build->>Build: Proceed with code-splitting
end
Build->>Virt: Generate virtual component/loader modules
Build->>Virt: Remove prior warning injections
note over Build,FS: Dead code elimination and transform complete
App->>FS: Import route module at runtime
alt Non-production & warning injected
FS-->>App: console.warn with non-splittable export names
else Production or no conflicts
FS-->>App: No warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 0808152
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
SeanCassiere
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader.tsx (1)
1-1: Pluralization nit when only one export is listedMessage reads “These exports …” but only “loaderFn” is listed. Consider singularizing when count === 1 for polish.
Illustrative tweak:
-console.warn("[tanstack-router] These exports from \"retain-exports-loader.tsx\" will not be code-split and will increase your bundle size:\n- loaderFn\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); +console.warn("[tanstack-router] This export from \"retain-exports-loader.tsx\" will not be code-split and will increase your bundle size:\n- loaderFn\nFor the best optimization, this item should either have its export statement removed, or be imported from another location that is not a route file.");packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component.tsx (1)
1-1: Nit: singular vs plural in messageWhen only one identifier is listed, consider “This export …” instead of “These exports …”. Not a blocker; callsite-wide change would touch many snapshots.
packages/router-plugin/src/core/code-splitter/compilers.ts (2)
87-88: Unnecessary variable assignmentThe intermediate
resultvariable doesn't add value here since you're immediately returning it.- const result = `${bareFilename}?${params.toString()}` - return result + return `${bareFilename}?${params.toString()}`
434-435: Consider using a more specific environment checkThe current check
process.env.NODE_ENV !== 'production'might not cover all production-like environments (e.g., 'staging', 'test'). Consider using a more explicit flag or checking for multiple production-like values.- if (process.env.NODE_ENV !== 'production') { + const isProdLike = ['production', 'staging'].includes(process.env.NODE_ENV || '') + if (!isProdLike) {Alternatively, you could use a dedicated environment variable like
TANSTACK_ROUTER_DEV_WARNINGSfor more explicit control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (71)
docs/router/framework/react/guide/automatic-code-splitting.md(1 hunks)packages/router-plugin/src/core/code-splitter/compilers.ts(6 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@component.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@notFoundComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-loader.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-loader@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-loader@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure@component---errorComponent---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure@loader.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader@errorComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure.tsx(1 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure@component---loader---notFoundComponent---pendingComponent.tsx(0 hunks)packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure@errorComponent.tsx(0 hunks)
💤 Files with no reviewable changes (49)
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-loader@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-export-component@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function@component---loader---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-loader@component---errorComponent---notFoundComponent---pendingComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-loader@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function@errorComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure@notFoundComponent.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function@loader.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound@component.tsx
- packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound@loader.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (17)
packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-loader.tsx (2)
1-1: Reinstated warning matches PR intent — LGTMThe warning surfaces clearly at module load and names the offending export (“loaderFn”), aligning with the PR objective to restore the notice.
1-1: Guard console.warn behind a dev-only checkUnconditional top-level console.warn risks shipping to production and preventing DCE — ensure the generator only emits this warning in dev builds (e.g. wrap with process.env.NODE_ENV !== 'production' or import.meta.env.DEV).
Re-check the plugin (not the snapshot). My earlier rg run failed with "unrecognized file type: tsx" — re-run these commands (no --type filters) and attach output if the guard is missing:
#!/bin/bash set -euo pipefail rg -nC2 '\[tanstack-router\].*will not be code-split' packages/router-plugin || true rg -nC2 'console\.warn.*will not be code-split' packages/router-plugin || true rg -nC3 'process\.env\.NODE_ENV|import\.meta\.env\.(DEV|PROD)' packages/router-plugin || true rg -n '\[tanstack-router\].*will not be code-split' packages/router-plugin/tests -g '!**/node_modules/**' || truepackages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const.tsx (1)
1-1: LGTM: correct identifiers listedListings for Layout and loaderFn align with the “components combined + loader separate” scenario.
packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component-and-normal-notFound.tsx (1)
1-1: LGTM: warns on default export used as componentThe “Home” export is correctly surfaced.
packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-function.tsx (1)
1-1: LGTM: only component warned under default behaviorLoader isn’t split by default here, so excluding
loaderFnfrom the warning is expected.packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-export-component.tsx (1)
1-1: LGTM: message accurately flags exported component
Layoutis exported while loader is lazy-split; messaging matches configuration.packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component.tsx (1)
1-1: LGTM: correct warning for default export
Homebeing exported explains the non-splittable warning.packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component.tsx (1)
1-1: Move injected warning below directive prologues/imports and guard it for dev-onlyFile not found: packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/export-default-component.tsx — confirm path or update the compiler/injector that emits this warning.
- Insert the console.warn after any directive prologue (e.g., "use client"/"use server") and after imports — AST-aware insertion.
- Wrap the warning in a dev-only guard (compile-time DEV or if (process.env.NODE_ENV !== 'production')) so production bundles contain no side effects and tree-shaking remains effective.
packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound.tsx (1)
1-1: ```shell
#!/bin/bash
set -euo pipefailFILE="packages/router-plugin/tests/code-splitter/snapshots/react/1-default/export-default-component-and-normal-notFound.tsx"
echo "=== CHECK FILE: $FILE ==="
if [ -f "$FILE" ]; then
sed -n '1,200p' "$FILE" || true
else
echo "File not found: $FILE"
echo "Listing directory contents (if any):"
ls -la packages/router-plugin/tests/code-splitter/snapshots/react/1-default || true
fiecho
echo "=== SEARCH FOR WARNING STRING ACROSS REPO ==="
rg -n --hidden --no-ignore -S "will not be code-split and will increase your bundle size" || true
rg -n --hidden --no-ignore -S "These exports from" || trueecho
echo "=== SEARCH FOR console.warn USAGES ==="
rg -n --hidden --no-ignore -S "console\.warn\(" || trueecho
echo "=== SEARCH FOR console.warn IN packages/router-plugin ==="
rg -n --hidden --no-ignore -S "console\.warn\(" packages/router-plugin || trueecho
echo "=== SEARCH FOR NODE_ENV or DEV (context) ==="
rg -n --hidden --no-ignore -S "NODE_ENV|DEV|console\.warn\(" packages || trueecho
echo "=== GIT BRANCH & TOP-LEVEL LISTING ==="
git rev-parse --abbrev-ref HEAD || true
ls -la | sed -n '1,120p' || true</blockquote></details> <details> <summary>packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function.tsx (1)</summary><blockquote> `1-1`: **Move dev-only console.warn after directive prologues/imports and guard it from production.** Place the warning after any "use ..." prologue and imports; wrap in a dev-only check (e.g., if (process.env.NODE_ENV !== 'production') { console.warn(... + ' [TSR001]'); }) and append a short docs/code id (TSR001) to the message. Location: packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-function.tsx:1 — file not found in repo during verification; confirm path. </blockquote></details> <details> <summary>packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component.tsx (1)</summary><blockquote> `1-1`: **Runtime warning correctly added for non-splittable exports** The console warning properly identifies that the `Home` export will not be code-split. This aligns with the PR's goal of restoring these warnings. </blockquote></details> <details> <summary>packages/router-plugin/src/core/code-splitter/compilers.ts (3)</summary><blockquote> `120-121`: **Good implementation of export tracking** The code correctly tracks exported identifiers that cannot be code-split by adding them to `knownExportedIdents` when `isExported` is true. This enables proper warning generation later. Also applies to: 257-259, 329-331 --- `422-440`: **Well-structured warning generation with proper dev/prod handling** The implementation correctly: 1. Checks if there are any non-splittable exports 2. Generates a user-friendly warning message via `createNotExportableMessage` 3. Logs to console for developer visibility 4. Only injects runtime warnings in non-production builds --- `847-860`: **Clean and informative warning message generation** The `createNotExportableMessage` function generates clear, actionable warning messages that: - List all problematic exports - Explain the performance impact - Provide clear guidance on how to fix the issue </blockquote></details> <details> <summary>packages/router-plugin/tests/code-splitter/snapshots/react/1-default/useStateDestructure.tsx (1)</summary><blockquote> `1-1`: **Test snapshot correctly updated with warning** The warning message properly identifies `VersionIndex` as a non-splittable export and provides clear guidance for optimization. </blockquote></details> <details> <summary>packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/useStateDestructure.tsx (1)</summary><blockquote> `1-1`: **Consistent warning implementation across test scenarios** The warning is consistently applied across different code-splitting configurations, ensuring proper coverage. </blockquote></details> <details> <summary>packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/useStateDestructure.tsx (1)</summary><blockquote> `1-1`: **Test coverage complete for all splitting scenarios** The warning implementation is consistent across all three test scenarios (1-default, 2-components-combined, 3-all-combined), demonstrating comprehensive test coverage. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
|
||
| ### Rules of Splitting | ||
|
|
||
| For automatic code splitting to work, there are some rules in-place to make sure that this process can reliably and predictably happen. |
There was a problem hiding this comment.
Fix minor grammar: “in-place” → “in place”
Improves readability in the new rules section.
Apply:
-For automatic code splitting to work, there are some rules in-place to make sure that this process can reliably and predictably happen.
+For automatic code splitting to work, there are some rules in place to make sure that this process can reliably and predictably happen.📝 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
| For automatic code splitting to work, there are some rules in-place to make sure that this process can reliably and predictably happen. | |
| For automatic code splitting to work, there are some rules in place to make sure that this process can reliably and predictably happen. |
🤖 Prompt for AI Agents
In docs/router/framework/react/guide/automatic-code-splitting.md around line 79,
the phrase "in-place" is grammatically incorrect; change "in-place" to "in
place" so the sentence reads "For automatic code splitting to work, there are
some rules in place to make sure that this process can reliably and predictably
happen." Ensure only that hyphen is removed and spacing is correct.
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"retain-exports-const.tsx\" will not be code-split and will increase your bundle size:\n- Layout\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Same: keep directives first and make the warning dev-only.
Move after directives/imports and guard behind a compile-time DEV flag for DCE.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const.tsx'
echo "=== FILE EXISTS ==="
if [ -f "$FILE" ]; then
echo "FOUND: $FILE"
else
echo "MISSING: $FILE"
fi
echo
echo "=== FILE CONTENT (first 200 lines) ==="
nl -ba "$FILE" | sed -n '1,200p' || true
echo
echo "=== SEARCH: warning text across repo ==="
rg -n --hidden --no-ignore "These exports from" || true
echo
echo "=== SEARCH: DEV flag patterns across repo (context) ==="
rg -n -C3 --hidden "__DEV__|process.env.NODE_ENV|import.meta.env.DEV|if \\(process.env.NODE_ENV|process.env.NODE_ENV !== 'production'" || true
echo
echo "=== SEARCH: 'use client' directives in repo (quick check) ==="
rg -n --hidden "'use client'|\"use client\"" || trueLength of output: 9539
🏁 Script executed:
#!/bin/bash
set -euo pipefail
COMPILERS='packages/router-plugin/src/core/code-splitter/compilers.ts'
SNAP1='packages/router-plugin/tests/code-splitter/snapshots/react/1-default/retain-exports-const.tsx'
SNAP2='packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-const.tsx'
echo "=== COMPILERS ==="
if [ -f "$COMPILERS" ]; then
echo "FOUND: $COMPILERS"
echo "---- lines 820-900 ----"
sed -n '820,900p' "$COMPILERS" || true
else
echo "MISSING: $COMPILERS"
fi
echo
echo "=== SNAPSHOT 1 ==="
if [ -f "$SNAP1" ]; then
echo "FOUND: $SNAP1"
sed -n '1,120p' "$SNAP1" || true
else
echo "MISSING: $SNAP1"
fi
echo
echo "=== SNAPSHOT 2 ==="
if [ -f "$SNAP2" ]; then
echo "FOUND: $SNAP2"
sed -n '1,160p' "$SNAP2" || true
else
echo "MISSING: $SNAP2"
fi
echo
echo "=== DEV-GATING PATTERNS ==="
rg -n --hidden "process.env.NODE_ENV" || true
rg -n --hidden "import.meta.env.DEV" || true
rg -n --hidden "__DEV__" || true
echo
echo "=== 'use client' DIRECTIVES ==="
rg -n --hidden "'use client'|\"use client\"" || trueLength of output: 11088
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='packages/router-plugin/src/core/code-splitter/compilers.ts'
echo "=== repo-wide console.warn occurrences ==="
rg -n "console\\.warn" || true
echo
echo "=== compilers.ts: search for warn/unshiftContainer/unshift/addDirective/directives ==="
rg -n --hidden "console\\.warn|unshiftContainer|unshift\\(|addDirective|directives" "$FILE" || true
echo
echo "=== compilers.ts (1-240) ==="
sed -n '1,240p' "$FILE" || true
echo
echo "=== compilers.ts (240-520) ==="
sed -n '240,520p' "$FILE" || true
echo
echo "=== compilers.ts (520-960) ==="
sed -n '520,960p' "$FILE" || trueLength of output: 46879
Same: keep directives first and make the warning dev-only.
Remove the unconditional console.warn and stop using ast.program.body.unshift so the warning doesn't appear before module directives/imports. Instead: (1) guard both the immediate log and the injected AST node with a compile-time DEV check (e.g. process.env.NODE_ENV !== 'production' or equivalent) so bundlers can DCE it; (2) insert the injected console.warn after any "use client" directives and after all ImportDeclaration nodes (compute an insert index and splice into ast.program.body).
Location: packages/router-plugin/src/core/code-splitter/compilers.ts (around the createNotExportableMessage / console.warn + ast.program.body.unshift section).
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"export-default-component-and-normal-notFound.tsx\" will not be code-split and will increase your bundle size:\n- Home\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Do not precede potential "use client"/"use server" with runtime code; dev-gate the warn.
Move the injected warn below any directives/imports and wrap with a dev check so it’s tree-shaken from production.
Move dev-only warn below directives/imports and gate it
Runtime code must not precede "use client"/"use server". Move the console.warn below any directives/imports and wrap it in a dev-only guard (e.g. if (process.env.NODE_ENV !== 'production') { ... }) so it is tree-shaken from production.
File: packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound.tsx (line 1)
console.warn("[tanstack-router] These exports from \"export-default-component-and-normal-notFound.tsx\" will not be code-split and will increase your bundle size:\n- Home\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file.");
🤖 Prompt for AI Agents
In
packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/export-default-component-and-normal-notFound.tsx
around line 1, the console.warn is emitted at the top-level before any module
directives/imports; move that warn so it appears after any "use client"/"use
server" directives and after all imports, and wrap it in a dev-only guard (for
example check NODE_ENV !== 'production') so the warning is tree-shaken from
production builds.
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"retain-exports-function.tsx\" will not be code-split and will increase your bundle size:\n- Layout\n- loaderFn\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Relocate + dev-gate the console.warn.
Ensure the compiler inserts after any "use client"/"use server" and that prod bundles don’t keep the side effect.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo ">> 1) Search repo for the exact warning text"
rg -n --hidden --no-ignore-vcs --color=never "\[tanstack-router\] These exports from" || true
echo
echo ">> 2) Search repo for files/occurrences named 'retain-exports-function'"
rg -n --hidden --no-ignore-vcs --color=never "retain-exports-function" || true
echo
echo ">> 3) Show the snapshot file (if present)"
SNAP="packages/router-plugin/tests/code-splitter/snapshots/react/2-components-combined-loader-separate/retain-exports-function.tsx"
if [ -f "$SNAP" ]; then
echo "---- $SNAP ----"
sed -n '1,200p' "$SNAP"
else
echo "File not found: $SNAP"
fi
echo
echo ">> 4) Search for console.warn usages in packages/router-plugin"
rg -n --hidden --no-ignore-vcs --color=never "console\\.warn" packages/router-plugin || true
echo
echo ">> 5) Search for code that constructs the message or references exports retention (keywords)"
rg -n --hidden --no-ignore-vcs --color=never "retain-exports|retainExports|will not be code-split|These exports from|will not be code-split" packages || true
echo
echo ">> 6) Search for common dev-gate patterns near those files (NODE_ENV, __DEV__, import.meta.env)"
rg -n --hidden --no-ignore-vcs --color=never "process\\.env\\.NODE_ENV|__DEV__|import\\.meta\\.env|if\\s*\\(process\\.env\\.NODE_ENV|if\\s*\\(__DEV__" packages/router-plugin || true
echo
echo ">> 7) List files under packages/router-plugin that may generate snapshots for code-splitting tests (top level)"
ls -la packages/router-plugin || trueLength of output: 48271
Relocate and dev‑gate the injected console.warn (prevent prod side‑effects).
The compiler currently injects an unconditional console.warn into emitted route files (snapshots show it as the first line), which can leak into production bundles.
- Edit packages/router-plugin/src/core/code-splitter/compilers.ts (see ~lines 431–436 and ~854) so the generated warning is inserted after any "use client"/"use server" prologue.
- Do not emit an unguarded console.warn into the module; instead emit a dev-only guard (e.g. if (process.env.NODE_ENV !== 'production') { console.warn(...); }) or stop emitting the warning into the emitted file and keep the compiler-side dev log. This ensures bundlers can remove the side‑effect for production.
🤖 Prompt for AI Agents
In packages/router-plugin/src/core/code-splitter/compilers.ts around the
existing insertion points (~lines 431–436 and ~854), the compiler currently
injects an unconditional console.warn at the top of emitted route modules;
relocate the injected warning so it is inserted after any "use client" / "use
server" prologue detection logic, and stop emitting an unguarded side-effect by
either wrapping the emitted warning in a dev-only guard (if
(process.env.NODE_ENV !== 'production') { console.warn(...) }) or by not writing
the warning into emitted modules at all and instead logging from the compiler
process; ensure the prologue-aware insertion preserves AST/source offsets so the
prologue remains the first statement and the warning is wrapped or moved as
described.
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"retain-exports-loader.tsx\" will not be code-split and will increase your bundle size:\n- loaderFn\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Warning content looks right; please verify non-prod gating and HMR dedupe
Confirm the compiler injects this only outside production and avoids duplicate logs on HMR/re-imports. Consider including the routeId or full file path in the message for quicker triage.
Run to verify gating and injection sites:
🏁 Script executed:
#!/usr/bin/env bash
# Check for gating and message construction in the plugin sources
rg -n -C2 "(createNotExportableMessage|console\\.warn|NODE_ENV|import\\.meta\\.env\\.PROD)" packages/router-plugin/srcLength of output: 2677
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "---- rg matches ----"
rg -n -C3 "knownExportedIdents.size > 0|createNotExportableMessage|warningTemplate|ast.program.body.unshift|console.warn\(" packages/router-plugin/src/core/code-splitter/compilers.ts || true
echo
echo "---- lines 400-480 ----"
sed -n '400,480p' packages/router-plugin/src/core/code-splitter/compilers.ts || true
echo
echo "---- lines 820-920 (createNotExportableMessage) ----"
sed -n '820,920p' packages/router-plugin/src/core/code-splitter/compilers.ts || trueLength of output: 6217
Gate build-time warn and make injected warning idempotent
- Wrap the compile-time console.warn so it's only emitted in non-production (process.env.NODE_ENV !== 'production') — currently console.warn(warningMessage) is unconditional in packages/router-plugin/src/core/code-splitter/compilers.ts (around lines 423–431).
- Prevent duplicate injected warnings on HMR/re-compiles by checking the AST for an existing injected console.warn (or using a unique sentinel/comment) before calling ast.program.body.unshift(warningTemplate) (injection occurs around lines 434–438).
- Ensure the message includes a clear triage key (routeId or absolute file path); createNotExportableMessage currently receives filename (function starts around line 847) — make it absolute or append routeId if available.
🤖 Prompt for AI Agents
In packages/router-plugin/src/core/code-splitter/compilers.ts around lines
423–438 and the createNotExportableMessage function around line 847, wrap the
compile-time console.warn injection so it runs only when process.env.NODE_ENV
!== 'production'; before injecting the warning AST node, scan the AST (e.g.,
program.body) for an existing injected sentinel (unique comment or a uniquely
phrased console.warn call) and skip injection if present to make the warning
idempotent across HMR/re-compiles; finally update createNotExportableMessage to
include a clear triage key by using the absolute file path (resolve to an
absolute path) and append routeId when available so the emitted message contains
routeId or absolute path for easier debugging.
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"retain-export-component.tsx\" will not be code-split and will increase your bundle size:\n- Layout\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Warning should not be the first statement; add dev guard.
Keep directive prologues intact and strip in production builds via a compile-time dev check.
Wrap the dev-only warning behind a compile-time dev guard
Do not leave console.warn as the first top-level statement — guard it with a build-time dev check (e.g., if (process.env.NODE_ENV !== 'production') or a DEV constant) so it is stripped from production, and keep any directive prologues (e.g., "use client") at the very top.
File: packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component.tsx:1
🤖 Prompt for AI Agents
In
packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-export-component.tsx
around line 1, the top-level console.warn must be guarded so it is removed in
production and any directive prologues remain at the very top; wrap the
console.warn call in a compile-time dev check (for example if
(process.env.NODE_ENV !== 'production') { ... } or if (__DEV__) { ... }) and
ensure any "use ..." directive prologues are placed before this guard so the
prologue stays first and the warning gets stripped from production builds.
| @@ -1,3 +1,4 @@ | |||
| console.warn("[tanstack-router] These exports from \"retain-exports-const.tsx\" will not be code-split and will increase your bundle size:\n- Layout\n- loaderFn\nFor the best optimization, these items should either have their export statements removed, or be imported from another location that is not a route file."); | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Place warning after directives/imports and gate by environment.
Same concerns: preserve top-of-file directives, and avoid retaining dev-only side effects in prod.
Additionally consider deduping by filename to prevent HMR spam (e.g., track warned files on globalThis in dev).
Place warning after directives/imports and gate by environment.
Move the top-level console.warn so it runs after any file directives (e.g., "use client"/"use server") and all imports, and wrap it in a dev-only guard (e.g., if (process.env.NODE_ENV !== 'production' || import.meta.env.DEV) { ... }). Deduplicate warnings per filename in dev to avoid HMR spam (e.g., track warned files on globalThis). File: packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const.tsx:1
🤖 Prompt for AI Agents
In
packages/router-plugin/tests/code-splitter/snapshots/react/3-all-combined-errorComponent-separate/retain-exports-const.tsx
around line 1, the top-level console.warn should be moved so it executes only
after any file directives (e.g., "use client"/"use server") and all import
statements, wrapped in a dev-only guard (for example check process.env.NODE_ENV
!== 'production' or import.meta.env.DEV), and deduplicated per filename to avoid
HMR spam by tracking warned files on a globalThis set (add filename to the set
before warning and skip if already present). Ensure the warning logic runs after
directives/imports, is gated to dev builds, and returns early when the file has
already been warned.
LadyBluenotes
pushed a commit
to LadyBluenotes/router
that referenced
this pull request
Sep 19, 2025
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
At some point in the refactors, the warning shown about the non-splittable exports was broken. This PR adds it back in.
Summary by CodeRabbit