fix(kit): add forward compatible nitro v3 types#34053
Conversation
|
|
WalkthroughThis pull request modifies the Nitro type compatibility layer in the Kit package to support both NitroV2 and NitroV3 simultaneously. A new Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/kit/src/nitro-types.ts (1)
16-20: Consider reformatting for readability.The nested ternary expressions are functionally correct but quite long. Consider reformatting with line breaks to improve maintainability.
♻️ Suggested formatting
-export type Nitro = isNitroV2 extends true ? isNitroV3 extends true ? NitroV2.Nitro | NitroV3.Nitro : NitroV2.Nitro : NitroV3.Nitro -export type NitroDevEventHandler = isNitroV2 extends true ? isNitroV3 extends true ? NitroV2.NitroDevEventHandler | NitroV3.NitroDevEventHandler : NitroV2.NitroDevEventHandler : NitroV3.NitroDevEventHandler -export type NitroEventHandler = isNitroV2 extends true ? isNitroV3 extends true ? NitroV2.NitroEventHandler | NitroV3.NitroEventHandler : NitroV2.NitroEventHandler : NitroV3.NitroEventHandler -export type NitroRouteConfig = isNitroV2 extends true ? isNitroV3 extends true ? NitroV2.NitroRouteConfig | NitroV3.NitroRouteConfig : NitroV2.NitroRouteConfig : NitroV3.NitroRouteConfig -export type NitroRuntimeConfig = isNitroV2 extends true ? isNitroV3 extends true ? NitroV2.NitroRuntimeConfig | NitroV3.NitroRuntimeConfig : NitroV2.NitroRuntimeConfig : NitroV3.NitroRuntimeConfig +export type Nitro = isNitroV2 extends true + ? isNitroV3 extends true ? NitroV2.Nitro | NitroV3.Nitro : NitroV2.Nitro + : NitroV3.Nitro + +export type NitroDevEventHandler = isNitroV2 extends true + ? isNitroV3 extends true ? NitroV2.NitroDevEventHandler | NitroV3.NitroDevEventHandler : NitroV2.NitroDevEventHandler + : NitroV3.NitroDevEventHandler + +export type NitroEventHandler = isNitroV2 extends true + ? isNitroV3 extends true ? NitroV2.NitroEventHandler | NitroV3.NitroEventHandler : NitroV2.NitroEventHandler + : NitroV3.NitroEventHandler + +export type NitroRouteConfig = isNitroV2 extends true + ? isNitroV3 extends true ? NitroV2.NitroRouteConfig | NitroV3.NitroRouteConfig : NitroV2.NitroRouteConfig + : NitroV3.NitroRouteConfig + +export type NitroRuntimeConfig = isNitroV2 extends true + ? isNitroV3 extends true ? NitroV2.NitroRuntimeConfig | NitroV3.NitroRuntimeConfig : NitroV2.NitroRuntimeConfig + : NitroV3.NitroRuntimeConfig
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/kit/src/nitro-types.tspackages/kit/src/nitro.tspackages/kit/src/pages.tspackages/kit/src/runtime-config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/kit/src/nitro.tspackages/kit/src/pages.tspackages/kit/src/runtime-config.tspackages/kit/src/nitro-types.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/kit/src/nitro.tspackages/kit/src/pages.tspackages/kit/src/runtime-config.tspackages/kit/src/nitro-types.ts
🧠 Learnings (2)
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
Applied to files:
packages/kit/src/pages.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/kit/src/pages.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). (14)
- GitHub Check: test-fixtures (windows-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: code
🔇 Additional comments (4)
packages/kit/src/nitro-types.ts (1)
10-14: LGTM!The
isNitroV3type guard correctly mirrors the existingisNitroV2pattern, checking for the presence of a validNitrotype by verifying theoptionskey exists and the___INVALIDmarker is absent.packages/kit/src/nitro.ts (1)
36-38: Acceptable trade-off for cross-version compatibility.The
as anycast mirrors the existing pattern inaddServerHandler(line 29) and is a reasonable workaround for the union type complexity introduced by dual Nitro version support.packages/kit/src/runtime-config.ts (1)
31-35: Appropriate type relaxation for forward compatibility.Casting to
Record<string, any>removes the dependency onNitroRuntimeConfigwhich may differ between Nitro v2 and v3. The existing try/catch properly handles the uninitialised Nitro case.packages/kit/src/pages.ts (1)
25-28: Consistent type handling for dual-version support.The
as anycasts are necessary to avoiddefutype conflicts whenNitroRouteConfigresolves to a union type. Runtime behaviour remains unchanged.
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
🔗 Linked issue
📚 Description
extracted from #33005