Conversation
📝 WalkthroughWalkthroughThe change replaces Rolldown feature detection with a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 9m 56s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 30s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-19 16:35:23 UTC
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-plugin-core/tests/utils.test.ts (1)
5-17: Good coverage of core version detection scenarios.The tests cover the primary use cases well. Consider adding a few edge case tests for completeness:
🧪 Optional: Additional edge case tests
test('defaults to rollup config keys for unknown versions', () => { expect(usesRolldown('unknown')).toBe(false) }) + + test('handles empty version string', () => { + expect(usesRolldown('')).toBe(false) + }) + + test('handles pre-release versions', () => { + expect(usesRolldown('8.0.0-beta.1')).toBe(true) + }) + + test('handles future major versions', () => { + expect(usesRolldown('9.0.0')).toBe(true) + }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/tests/utils.test.ts` around lines 5 - 17, Add edge-case unit tests for the usesRolldown utility to cover boundary and non-semver inputs: add tests calling usesRolldown with pre-release/patch variants like "8.0.0-beta.1" and "7.9.9", with a null/undefined input and an empty string to ensure defaults, and a malformed semver like "v8" to verify behavior; update the tests in tests/utils.test.ts around the describe('usesRolldown', ...) block so the new cases assert the expected boolean results for each input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/start-plugin-core/tests/utils.test.ts`:
- Around line 5-17: Add edge-case unit tests for the usesRolldown utility to
cover boundary and non-semver inputs: add tests calling usesRolldown with
pre-release/patch variants like "8.0.0-beta.1" and "7.9.9", with a
null/undefined input and an empty string to ensure defaults, and a malformed
semver like "v8" to verify behavior; update the tests in tests/utils.test.ts
around the describe('usesRolldown', ...) block so the new cases assert the
expected boolean results for each input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42dbe3a7-4cb3-44cb-be50-2a22f455e7e2
📒 Files selected for processing (2)
packages/start-plugin-core/src/utils.tspackages/start-plugin-core/tests/utils.test.ts
![chatgpt-codex-connector[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a366e85a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * to `build.rolldownOptions`. | ||
| */ | ||
| export const isRolldown = 'rolldownVersion' in vite | ||
| export const isRolldown = usesRolldown(vite.version) |
There was a problem hiding this comment.
usesRolldown(vite.version) still looks up the vite package resolved next to @tanstack/start-plugin-core, not the Vite instance that is actually executing the build. In this repo packages/start-plugin-core is pinned to Vite 8 (packages/start-plugin-core/package.json / pnpm-lock.yaml), while benchmarks/ssr explicitly installs Vite 7.3.1, so the React/Solid/Vue SSR benchmarks this commit is trying to restore will still see 8.x here and keep writing build.rolldownOptions. Under Vite 7 that key is ignored, so plugin.ts falls back to the broken HTML entry resolution path again instead of using the server alias.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing 
Footnotes
|
1 participant
Summary
rollupOptionsinstead of incorrectly switching torolldownOptionsindex.htmlentry resolution in React, Solid, and VueTesting
node_modulesis not installed)Summary by CodeRabbit
Release Notes
New Features
Tests