fix: keep non-picked route exports that picked exports reference#2205
Open
brenelz wants to merge 6 commits into
Open
fix: keep non-picked route exports that picked exports reference#2205brenelz wants to merge 6 commits into
brenelz wants to merge 6 commits into
Conversation
The route tree-shaker removed the entire binding of any export not in
the pick list, so an API handler calling a helper exported from the
same file crashed at runtime with a ReferenceError. Instead of deleting
the declaration, strip only the export keyword and register the kept
binding with the existing sweep pass, which still removes it (and its
imports) when nothing references it - so no extra code reaches a bundle
that didn't already reach it.
Also fixes an always-true condition in the export-specifier handling
that dropped picked exports declared via specifiers, e.g.
`export { POST as GET }`.
Mixed declarations (`export const a = ..., b = ...`) are split into one
declaration per declarator, preserving evaluation order, and only the
picked ones stay exported. Named default-export declarations are kept
in module scope when other exports reference them; anonymous ones are
still removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: c137f34 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for solid-start-landing-page ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
brenelz
commented
Jul 17, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
commit: |
lxsmnsyc
approved these changes
Jul 20, 2026
lxsmnsyc
left a comment
lxsmnsyc
left a comment
Member
There was a problem hiding this comment.
I'll approve this for now because of the tests, but I'll probably simplify this later
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes SolidStart’s fs-routes tree-shaker so that non-picked exports that are still referenced by picked exports are no longer deleted (avoiding runtime ReferenceErrors), and so that export { ... } specifier exports are filtered correctly by exported name.
Changes:
- Un-exports (strips
export) instead of deleting certain non-picked bindings, allowing picked exports (e.g.GET) to reference helpers defined in the same module. - Fixes
export { POST as GET }/ aliased specifier handling by filtering based on the exported name. - Adds a dedicated Vitest suite covering the reported regressions and key edge cases (default export behavior, mixed declarations, import sweeping).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/start/src/config/fs-routes/tree-shake.ts | Adjusts the Babel transform to keep referenced non-picked bindings in module scope and correctly filter export specifiers by exported name. |
| packages/start/src/config/fs-routes/tree-shake.spec.ts | Adds regression and behavior tests for keeping referenced helpers, alias specifiers, mixed declarations, default exports, and import cleanup. |
| .changeset/fix-2100-exports-disappear.md | Publishes a patch changeset describing the fixes for #2100 and #1659. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Fixes #2100
What is the current behavior?
The route tree-shaker deletes the entire binding of any export not in the
?picklist. An API route likeis imported with
?pick=GET,hello's declaration is removed, and the handler crashes at runtime withReferenceError: hello is not defined(500 on every request). Verified end-to-end on a production build of the basic fixture.There is also an always-true condition in the export-specifier handling that removes picked specifiers, so
export { POST as GET }never survives — the root cause of #1659.What is the new behavior?
Non-picked exports are un-exported instead of deleted: the
exportkeyword is stripped and the binding stays in module scope. Kept bindings are registered with the existing sweep pass, which still removes them (and their imports) when nothing references them — so no code reaches a bundle it didn't already reach, addressing the server-code-leak concern discussed in #2104.Details:
export const a = ..., GET = ...) are split into one declaration per declarator, preserving evaluation order; only picked ones stay exported.export { a, b as c }specifiers are now filtered by their exported name (fixes [Bug?]:export { POST as GET }is now broken #1659).export classis handled likeexport function.Other information
Added
packages/start/src/config/fs-routes/tree-shake.spec.ts(9 tests, 5 of which fail without this fix). Full playwright e2e suite inapps/testspasses. Supersedes the approach in #2104, which un-exported picked declarators in mixed declarations, produced invalid syntax for anonymous default exports, and kept unreferenced non-picked exports in the output.🤖 Generated with Claude Code