Repository navigation
Tanstack-react: Keep JSX-only component references during dead-code elimination - #35206
huang-julien merged 6 commits into
Conversation
…ode elimination The server-code-elimination plugin strips a route's server option and then runs a dead-code pass. Its reference collector only counted plain Identifier nodes, so a component used only inside JSX (<ImportedPanel />) — whether imported or declared in the same file — looked unreferenced and its import or declaration was removed, causing 'X is not defined' at runtime. Collect JSX element names (including the root of <Foo.Bar /> member expressions) as references so they are kept. Adds regression tests for imported, same-file, and member-expression JSX references. Fixes storybookjs#35175
📝 WalkthroughWalkthroughThe ChangesJSX Reference Preservation in Server-Code Elimination
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/frameworks/tanstack-react/src/plugins/server-code-elimination.ts`:
- Around line 414-416: The code at lines 414-416 adds all JSXIdentifiers to the
referenced set, including intrinsic HTML tags like div and span, which falsely
keeps unrelated lowercase bindings alive during elimination. Modify the
condition within the `t.isJSXIdentifier(name)` check to exclude intrinsic JSX
tags before adding to the referenced set. You can identify intrinsic tags by
checking if the identifier name starts with a lowercase letter or matches known
HTML tag patterns, and only add component-like identifiers (those starting with
uppercase or custom components) to the referenced set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 549bec17-52bb-4947-9300-b9b71decd7f4
📒 Files selected for processing (2)
code/frameworks/tanstack-react/src/plugins/server-code-elimination.test.tscode/frameworks/tanstack-react/src/plugins/server-code-elimination.ts
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.32 MB | 22.16 MB | 🚨 +835 KB 🚨 |
| Dependency size | 36.44 MB | 36.44 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 13 KB | 15 KB | 🚨 +2 KB 🚨 |
| Dependency size | 30.94 MB | 30.94 MB | 🎉 -694 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 533 | 533 | 0 |
| Self size | 640 KB | 663 KB | 🚨 +23 KB 🚨 |
| Dependency size | 62.27 MB | 62.31 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.37 MB | 1.39 MB | 🚨 +22 KB 🚨 |
| Dependency size | 23.97 MB | 24.01 MB | 🚨 +38 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 122 | 122 | 0 |
| Self size | 29 KB | 30 KB | 🚨 +970 B 🚨 |
| Dependency size | 25.04 MB | 25.08 MB | 🚨 +38 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 36 KB | 🚨 +4 KB 🚨 |
| Dependency size | 21.75 MB | 21.78 MB | 🚨 +35 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 274 | 274 | 0 |
| Self size | 23 KB | 23 KB | 🚨 +777 B 🚨 |
| Dependency size | 48.06 MB | 48.09 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 112 KB | 110 KB | 🎉 -2 KB 🎉 |
| Dependency size | 21.78 MB | 21.82 MB | 🚨 +38 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 205 | 0 |
| Self size | 827 KB | 827 KB | 🚨 +84 B 🚨 |
| Dependency size | 91.32 MB | 92.16 MB | 🚨 +835 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 32 KB | 32 KB | 🎉 -79 B 🎉 |
| Dependency size | 89.80 MB | 90.63 MB | 🚨 +835 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.09 MB | 1.09 MB | 🎉 -654 B 🎉 |
| Dependency size | 57.77 MB | 58.60 MB | 🚨 +835 KB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.49 MB | 1.52 MB | 🚨 +35 KB 🚨 |
| Dependency size | 12.45 MB | 12.45 MB | 🚨 +81 B 🚨 |
| Bundle Size Analyzer | Link | Link |
…-dead-import-35175 Tanstack-react: Keep JSX-only component references during dead-code elimination (cherry picked from commit 385f16a)
Closes #35175
What I did
Fixed a dead-code-elimination bug in
@storybook/tanstack-reactwhere components referenced only from JSX were wrongly removed, producing a runtimeReferenceError: X is not defined.The
server-code-eliminationVite plugin strips a route'sserveroption (server-only handlers/middleware) and then runs an iterative dead-code pass (removeDeadTopLevelDeclarations+removeDeadImportSpecifiers) to drop now-unused imports and declarations. That pass decides what is "still used" viacollectReferencedIdentifiers, which only collected plain BabelIdentifiernodes.In Babel's AST, a JSX element name such as
<ImportedPanel />is aJSXIdentifier(and<Foo.Bar />is aJSXMemberExpression), not anIdentifier. So any component used only inside JSX looked unreferenced and was removed:<ImportedPanel />) → itsimportgot deleted<NestedComponent />) → its top-level declaration got deletedThis only surfaced when a
serveroption was present, because that is what triggers the dead-code pass (state.modified).The fix adds a
JSXOpeningElementvisitor tocollectReferencedIdentifiersthat records the element's name as a reference, unwrapping member expressions to their root binding (UIin<UI.Panel />). Because both elimination passes share this collector, imported and same-file JSX-only references are now kept.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Added regression tests in
code/frameworks/tanstack-react/src/plugins/server-code-elimination.test.tscovering:<ImportedPanel />)<NestedComponent />, the follow-up case from the issue)<UI.Panel />)Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
A maintainer can reproduce the original bug and confirm the fix using the reporter's reproduction:
yarn task sandbox --template react-vite/default-ts --start-from auto(or use the reporter's repro: https://github.com/hollandjake/github-cqpvv6n3/tree/middleware-breaks-deadcode).serveroption (e.g. a middleware) and a component that renders a child imported into the file, e.g.:npm run storybook/yarn storybook) and open a story that renders this route.Story did something wrong : ReferenceError: ImportedPanel is not defined.After this fix: the story renders
<ImportedPanel />successfully.Documentation
MIGRATION.MD
No documentation changes needed — this is an internal bug fix with no API or behavior surface for users to configure.
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
serverconfiguration, ensuring they remain available for rendering.