fix(react-start): Do not optimizeDeps in VITEST environment - #6074
fix(react-start): Do not optimizeDeps in VITEST environment#6074onlywei wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughAdds a Vitest guard to React Start’s Vite plugin so dependency optimization is skipped during Vitest runs. Adds an E2E fixture with React routes, hooks, isomorphic execution, Vitest configuration, and tests validating the behavior. ChangesReact Start Vitest support
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 unit tests (beta)
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 9fae447
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between bcce02c and b025a578e6b18424367eee1db7b0ad9cdc8b40a8.
📒 Files selected for processing (1)
packages/react-start/src/plugin/vite.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/react-start/src/plugin/vite.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/react-start/src/plugin/vite.ts
🧠 Learnings (1)
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/react-start/src/plugin/vite.ts
⏰ 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: Test
- GitHub Check: Preview
|
Made a repro and an issue: #6246. The problem can be clearly seen there. This PR fixes it. |
3dc34cf to
79c06e4
Compare
|
Thanks @onlywei I hope @schiller-manuel is around today approve and merge. :-) |
He's most likely busy making awesome features like React Server Components work. I'll be patient and keep rebasing to resolve lockfile conflicts. |
Absolutely, and I'm using the workaround for now anyway. :-) |
Does |
We're not using any environment functions yet, so it's just the one basic server function using |
I see. Without the Tanstack start vite plugin enabled, it's hard for me to test any isomorphic function. |
b545524 to
9fae447
Compare
|
The repro and coverage here are useful, and #6246 is still open. The Vite plugin has changed since this was last updated, so could you rebase and apply the Vitest guard to the current needsOptimizeDeps logic? After that we can review it against the current plugin. |
9fae447 to
bd6b523
Compare
|
Note The previously reviewed commits are no longer reachable (likely due to a force-push or rebase), so CodeRabbit is performing a full review instead of an incremental one. This review may take a little longer. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/react-start/vitest-hooks/package.json (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
workspace:^instead ofworkspace:*for internal dependencies in e2e apps.While the general coding guidelines recommend
workspace:*, learnings specific to this repository indicate thatworkspace:^is the established convention for e2e test apps, whereasworkspace:*is reserved for published/library packages. Consider updating the workspace protocol to match the e2e convention.♻️ Proposed fix
"dependencies": { - "`@tanstack/react-router`": "workspace:*", - "`@tanstack/react-start`": "workspace:*", + "`@tanstack/react-router`": "workspace:^", + "`@tanstack/react-start`": "workspace:^", "react": "^19.2.3",🤖 Prompt for 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. In `@e2e/react-start/vitest-hooks/package.json` around lines 10 - 11, Update the internal dependency declarations for `@tanstack/react-router` and `@tanstack/react-start` in the package manifest to use the workspace:^ protocol, matching the established convention for e2e applications instead of workspace:*.Source: Learnings
🤖 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.
Nitpick comments:
In `@e2e/react-start/vitest-hooks/package.json`:
- Around line 10-11: Update the internal dependency declarations for
`@tanstack/react-router` and `@tanstack/react-start` in the package manifest to use
the workspace:^ protocol, matching the established convention for e2e
applications instead of workspace:*.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd05de0d-5569-4790-8af1-01d06a1d8d2e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
e2e/react-start/vitest-hooks/package.jsone2e/react-start/vitest-hooks/src/components/Counter.tsxe2e/react-start/vitest-hooks/src/isomorphic.tse2e/react-start/vitest-hooks/src/routeTree.gen.tse2e/react-start/vitest-hooks/src/router.tsxe2e/react-start/vitest-hooks/src/routes/__root.tsxe2e/react-start/vitest-hooks/src/routes/index.tsxe2e/react-start/vitest-hooks/tests/setup.tse2e/react-start/vitest-hooks/tests/vitest.spec.tsxe2e/react-start/vitest-hooks/tsconfig.jsone2e/react-start/vitest-hooks/vite.config.tspackages/react-start/src/plugin/vite.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-start/src/plugin/vite.ts
Why
Vitest + react-start currently forces
optimizeDepsto prebundle React, which can create dual React instances and break hooks. The workaround is disabling the plugin, but thencreateIsomorphicFn()is never transformed and becomes a no-op.Fixes #6246 and #6262
Dilemma
Without this fix, I have to give up using either
useState(),createIsomorphicFn(), or Vitest. I can’t use all three.Fix
Skip the aggressive
optimizeDepsconfiguration whenprocess.env.VITEST === 'true', so hooks work andcreateIsomorphicFn()still transforms.This is actually just a 1-line fix, the rest is just comments and an e2e test.
Verified
Vitest passes locally without workarounds.
Summary by CodeRabbit
Bug Fixes
Tests