fix: use h3 alias only in nitro-v2-plugin #5370
Conversation
avoid issues with nitro v3 see nitrojs/nitro#3613 (comment)
github-actions
bot
added
package: start-server-core
package: nitro-v2-vite-plugin
labels
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Vite as Vite/Rollup
participant Plugin as nitroV2Plugin (resolveId)
participant Node as Node (createRequire / require.resolve)
participant h3v1 as h3-v1 (npm:h3@^1.15.4)
Dev->>Vite: Start build for Nitro bundle
Vite->>Plugin: resolveId('h3', importer)
alt importer is nitropack-related or virtual entry
Plugin->>Node: require.resolve('h3-v1')
Node-->>Plugin: resolved path to h3-v1
Plugin-->>Vite: return resolved h3-v1 path
Vite->>h3v1: load resolved module
else other importers
Plugin-->>Vite: no mapping (defer)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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)
🔇 Additional comments (1)
Comment |
|
View your CI Pipeline Execution ↗ for commit edd4f13
☁️ 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nitro-v2-vite-plugin/src/index.ts (1)
118-130: Consider refining the resolution condition for precision.The custom
resolveIdhook correctly redirectsh3imports toh3-v1for Nitro v2 compatibility. However, the conditionimporter?.includes('nitropack')uses a broad string match that could match unintended paths.Consider making the condition more precise and adding an explanatory comment:
{ name: 'resolve', + // Redirect h3 imports to h3-v1 for Nitro v2 compatibility + // See: https://github.com/nitrojs/nitro/issues/3613 resolveId(source, importer) { if ( - (importer?.includes('nitropack') || + (importer?.includes('/nitropack/') || importer === virtualEntry) && source === 'h3' ) { return h3 } return null }, },The refined condition uses
/nitropack/to match path segments more precisely, reducing the risk of false positives. The comment documents the purpose and references the related issue.
📜 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(1 hunks)packages/nitro-v2-vite-plugin/src/index.ts(4 hunks)packages/nitro-v2-vite-plugin/vite.config.ts(1 hunks)packages/start-server-core/package.json(1 hunks)packages/start-server-core/src/request-response.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/start-server-core/src/request-response.tspackages/nitro-v2-vite-plugin/vite.config.tspackages/nitro-v2-vite-plugin/src/index.ts
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/src/request-response.tspackages/start-server-core/package.json
**/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
⏰ 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: Preview
- GitHub Check: Test
🔇 Additional comments (6)
packages/start-server-core/package.json (1)
71-71: LGTM! Version update from aliased h3-v2 to direct h3 package.The change from the alias
h3-v2to the directh3package at version2.0.0-beta.5is correct and aligns with the import updates inrequest-response.ts. The version bump includes new features like universal fetch handlers and middleware improvements.Since h3 v2 is still in beta, ensure that the version bump from beta.4 to beta.5 doesn't introduce breaking changes affecting your usage. Based on learnings, the beta.5 release includes middleware subsystem refactors and new handler types. Test critical code paths that use h3 session, cookie, and request/response utilities.
packages/start-server-core/src/request-response.ts (1)
22-22: LGTM! Import path correctly updated.The import path change from
h3-v2toh3aligns with the dependency update inpackage.json. All imported symbols remain unchanged, ensuring consistent behavior.packages/nitro-v2-vite-plugin/package.json (1)
64-65: LGTM! h3-v1 alias added for Nitro v2 compatibility.The
h3-v1alias dependency correctly pins h3 v1 for Nitro v2, allowing it to coexist with h3 v2 used elsewhere in the monorepo. The version range^1.15.4is appropriate for stable v1 releases.packages/nitro-v2-vite-plugin/vite.config.ts (1)
12-12: Verify consistency with package.json "main" field.Setting
cjs: falsedisables the CommonJS build, butpackage.jsonline 43 still declares"main": "dist/cjs/index.cjs". Since theexportsfield (lines 45-53) only provides animportcondition and the package requires Node >=22.12 (line 60), this package is effectively ESM-only.Consider either:
- Removing the
"main"and"module"fields frompackage.jsonif CJS support is not needed, or- Keeping
cjs: true(default) if legacy tooling compatibility is required.Given the modern Node.js requirement, removing the legacy fields is likely the cleaner approach.
packages/nitro-v2-vite-plugin/src/index.ts (2)
6-6: LGTM! Correct use of createRequire for ESM module resolution.The
createRequireimport and usage are appropriate for resolving theh3-v1module path in an ESM context.Also applies to: 19-19
97-97: LGTM! h3-v1 path resolution at build time.Resolving the
h3-v1path during the build phase ensures consistent module resolution for the customresolveIdhook.
This reverts commit 4d0e3cc.
avoid issues with nitro v3
see nitrojs/nitro#3613 (comment)
Summary by CodeRabbit
Bug Fixes
Chores