fix(start): preserve resolved virtual module ids#7844
Merged
Conversation
Contributor
|
View your CI Pipeline Execution ↗ for commit 1b8c994
☁️ Nx Cloud last updated this comment at |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThe Start compiler now preserves exact Vite module IDs, including virtual prefixes and query parameters, while server-function lookup flags are managed independently. New utilities, regression tests, and a build-only import-protection reproduction cover the updated behavior. ChangesModule identity preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 4 bumped as dependents. 🟩 Patch bumps
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@packages/start-plugin-core/src/vite/module-id.ts`:
- Around line 17-18: Update the module-ID flag appending logic around the
separator calculation so an ID ending with an empty query delimiter (`?`)
retains that delimiter through add/remove round trips; only use no separator
when the ID already ends with `&`, otherwise append `&`. Add a
`module-id.test.ts` case covering an ID ending in `?` and verifying the original
ID is restored after flag removal.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4cd54fab-b92b-4312-9455-0425cb753c6d
📒 Files selected for processing (12)
.changeset/tidy-pandas-load.mde2e/react-start/import-protection/src/routes/issue-7725.tsxe2e/react-start/import-protection/src/violations/issue-7725-server-fn-factory.tse2e/react-start/import-protection/vite.config.tspackages/start-plugin-core/src/import-protection/constants.tspackages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/src/start-compiler/utils.tspackages/start-plugin-core/src/vite/import-protection-plugin/plugin.tspackages/start-plugin-core/src/vite/module-id.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.tspackages/start-plugin-core/tests/vite/module-id.test.ts
💤 Files with no reviewable changes (1)
- packages/start-plugin-core/src/import-protection/constants.ts
Contributor
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will degrade performance by 13.2%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server error-paths unmatched (react) |
263.8 KB | 844.6 KB | -68.77% |
| ⚡ | Memory | mem server error-paths redirect (vue) |
732.1 KB | 431.4 KB | +69.71% |
| ⚡ | Memory | mem server error-paths not-found (react) |
276.7 KB | 266.3 KB | +3.93% |
| ⚡ | Simulation | ssr streaming deferred (react) |
73 ms | 70.9 ms | +3.03% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix-7725 (1b8c994) with main (62a191b)2
Footnotes
-
6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(31a634d) during the generation of this report, so 62a191b was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
schiller-manuel
force-pushed
the
fix-7725
branch
from
July 18, 2026 17:18
7bc436c to
1b8c994
Compare
Merged
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
1 participant
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 #7725
Summary by CodeRabbit