-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: revert alias handling for nitro-v2-vite-plugin #5378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughIntroduces a CommonJS entry for nitro-v2-vite-plugin, removes h3-v1 aliasing and resolve logic in its SSR build, and drops cjs override in its Vite config. start-server-core switches from direct h3 to an aliased h3-v2 dependency and updates corresponding imports. No public API shape changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Vite/Rollup
participant Plugin as nitro-v2-vite-plugin
participant Resolver as Module Resolver
Note over Dev,Resolver: SSR build entry resolution
rect rgba(200,230,255,0.25)
Note over Plugin: Previous flow
Dev->>Plugin: configure rollup
Plugin->>Resolver: resolveId hook (h3 → h3-v1)
Resolver-->>Dev: module id (h3-v1)
end
rect rgba(200,255,200,0.25)
Note over Plugin: New flow
Dev->>Plugin: configure rollup
Plugin->>Dev: plugins = [virtualBundlePlugin]
Dev->>Resolver: standard resolution (no h3 remap)
Resolver-->>Dev: module id (as declared)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit c5581b0
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nitro-v2-vite-plugin/src/index.ts (1)
117-119: Fix h3 import version mismatch in virtual entry (packages/nitro-v2-vite-plugin/src/index.ts:117–119)
nitropack depends on h3 v1.15.4 which doesn’t export fromWebHandler; this import will fail at runtime. Align your build on h3 v2 (e.g. update dependency or add an alias/peer-declaration for h3 v2).
🧹 Nitpick comments (1)
packages/start-server-core/package.json (1)
71-71: Verify the version downgrade from beta.5 to beta.4.The dependency was downgraded from
h3@2.0.0-beta.5toh3@2.0.0-beta.4. Please confirm this downgrade is intentional and document the reason (e.g., breaking changes in beta.5, compatibility issues with Bun, or other considerations).Additionally, consider the stability implications of depending on a beta version. If this is intended for production use, evaluate whether the beta status aligns with your stability requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/nitro-v2-vite-plugin/package.json(2 hunks)packages/nitro-v2-vite-plugin/src/index.ts(1 hunks)packages/nitro-v2-vite-plugin/vite.config.ts(0 hunks)packages/start-server-core/package.json(1 hunks)packages/start-server-core/src/request-response.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/nitro-v2-vite-plugin/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
packages/start-server-core/package.jsonpackages/nitro-v2-vite-plugin/package.json
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-server-core/package.jsonpackages/start-server-core/src/request-response.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/nitro-v2-vite-plugin/src/index.tspackages/start-server-core/src/request-response.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
🔇 Additional comments (4)
packages/start-server-core/src/request-response.ts (1)
22-22: LGTM! Import source correctly updated to h3-v2 alias.The change aligns with the PR objective to migrate away from h3-v1 aliasing. All imported functions are retained in h3 v2 with backward-compatible APIs.
Based on learnings
packages/nitro-v2-vite-plugin/package.json (2)
64-64: LGTM! Removal of h3-v1 dependency aligns with PR objective.Removing the h3-v1 alias resolves the Bun compatibility issue mentioned in the PR description, where Bun only installs h3@v1 and disregards the h3@v2 beta.
43-43: Verify CommonJS bundle output
Ensurevite buildactually generatesdist/cjs/index.cjs—for example, addbuild.lib.formats = ['cjs','es']inpackages/nitro-v2-vite-plugin/vite.config.ts—and confirm the file exists post-build.packages/nitro-v2-vite-plugin/src/index.ts (1)
113-113: LGTM! Simplified rollup plugin configuration.Removing the h3-v1 resolution logic aligns with the PR objective to revert the alias handling that didn't work with Bun.
|
I get this error with '@tanstack/nitro-v2-vite-plugin': ^1.132.40 |
the approach did not work with bun, bun only installs h3@v1 (it probably disregards v2 as it is still beta)
Summary by CodeRabbit
New Features
Refactor
Chores
Impact