feat: mock runtime virtual imports#3861
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@pi0 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughStandardizes public import/export aliases to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus review on:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/runtime/internal/virtual/routing.ts (1)
1-25: Internal routing stubs match the public virtual surfaceThe internal stubs are consistent with the public
src/runtime/virtual/routing.tsversion (same signatures and neutral defaults). See the comment on that file about potentially sharing a single implementation to avoid duplication; otherwise, this looks fine.src/runtime/virtual/public-assets.ts (1)
1-15: Same stub as internal public-assets; consider single implementation + re-exportThis file duplicates
src/runtime/internal/virtual/public-assets.tsline-for-line. To keep behavior in sync long-term, it may be cleaner to have only one of these define the stub and have the other do a straightexport * fromre-export, as suggested in the internal module comment.
🧹 Nitpick comments (13)
src/runtime/virtual/polyfills.ts (1)
1-1: Clarify intent of empty polyfills stubThe empty default export is fine as a stub to make
#nitro/virtual/polyfillsresolvable. To make its purpose clearer and avoid accidental use, consider adding a brief comment or a more explicit type, e.g.:// Stub polyfills module for runtime tests / non-bundled usage. export default {} as Record<string, never>;src/build/plugins/resolve.ts (1)
17-24: Virtual-import resolution change looks correct; consider tightening the importer checkThe new branch that re-resolves ids when
importer?.includes("#nitro/virtual")lets imports from virtual modules go through a second resolution pass rooted atruntimeDir, after which the subpath map (e.g.nitro/h3→h3) can still apply. That matches the goal of making the runtime work outside a bundle.If you want to be a bit more explicit, you could narrow the condition to something like
importer?.startsWith("#nitro/virtual/")and/or add a short comment explaining the two-phase resolution for imports coming from virtual modules, but the current logic is functionally sound.src/runtime/internal/virtual/error-handler.ts (1)
6-8: Internal virtual error handler stub is fine; consider underscore parametersThis stub gives you a simple, type-correct default that always returns a 500 response, which is reasonable for an internal fallback. To avoid unused-parameter lint noise while keeping the signature, you could rename the arguments to
_error/_event:const errorHandler: (error: EParams[0], event: EParams[1]) => EReturn = ( _error, _event, ) => new Response("Internal Server Error", { status: 500 });src/runtime/virtual/error-handler.ts (1)
1-17: Default virtual error handler behavior and robustnessThis gives
#nitro/virtual/error-handlera concrete, minimal implementation for unbundled runtime usage: non‑404 errors are logged and everything is turned into aResponseviatoResponse, which is a sensible default.Two small points to double‑check:
Custom error handlers vs stub behavior
The build‑time virtualerror-handlermodule wires in user‑providedNitroErrorHandlers, whereas this runtime stub directly callstoResponse. If the intent is that customnitro.options.errorHandlerhooks only apply in bundled builds, this is fine; otherwise, you may want a follow‑up to mirror more of that behavior in the stub as well.Defensive access to
error.status
In practiceerrorshould be anHTTPError, but for extra safety against unexpected throws (e.g. plainErroror other values), consider using optional chaining to avoid a crash in the logging path beforetoResponseruns:if ((error as any)?.status !== 404) { console.error(error as any); }Based on learnings, this should still coexist with adapter-specific error handling (e.g. Cloudflare’s preference for surfacing native error messages), but it’s worth confirming adapter behavior remains as expected.
src/runtime/internal/virtual/renderer-template.ts (1)
1-7: Consider wideningrendererTemplatereturn type to match generated virtual module
src/build/virtual/renderer-template.tscan emit arendererTemplatethat returns anHTTPResponse(static prod branch), whereas this stub advertises onlystring | Promise<string>. That makes it harder to write type‑safe code that handles the response object shape exposed at runtime.If you want the stub typings to reflect the full surface, consider widening the return type, for example:
+import type { HTTPResponse } from "h3"; -export function rendererTemplate(_req: Request): string | Promise<string> { +export function rendererTemplate( + _req: Request +): string | Promise<string> | HTTPResponse { return `<!-- Renderer template not available -->`; }(or another union that matches your intended long‑term API). If consumers never rely on the response object and always normalize this value elsewhere, the current type is fine and this can be skipped.
src/runtime/virtual/tasks.ts (1)
1-8: Clarify the default forscheduledTasksvs itsfalse | []unionThe type
false | { cron: string; tasks: string[] }[]suggestsfalseis a special “scheduler disabled/not configured” sentinel, while this stub defaults to[](scheduler enabled but with no entries).If other code distinguishes between “no scheduled tasks” and “scheduler off” via a strict
=== falsecheck, you might want the stub to mirror that by defaulting tofalseinstead:-export const scheduledTasks: false | { cron: string; tasks: string[] }[] = []; +export const scheduledTasks: false | { cron: string; tasks: string[] }[] = false;If no such distinction exists and
[]is sufficient, current code is fine.src/runtime/virtual/routing.ts (1)
1-25: Consider sharing routing stubs between public and internal virtual modulesThis file and
src/runtime/internal/virtual/routing.tsdefine the same four no‑op exports with identical signatures. To reduce duplication and keep the surfaces in sync, you could have one re‑export from the other, for example:// In src/runtime/virtual/routing.ts export { findRoute, findRouteRules, globalMiddleware, findRoutedMiddleware, } from "../internal/virtual/routing";(or invert the direction if you prefer public → internal). Not critical, but it would make future changes to the stub surface less error‑prone.
src/runtime/virtual/feature-flags.ts (1)
1-7: Feature flag surface looks consistent and type-safeThe exported flags are straightforward and match the internal mirror, giving a clear, typed feature-detection surface under
#nitro/virtual/feature-flags. No correctness issues from what’s shown.If you find these drifting over time between internal/public, consider centralizing via a shared module or re-export to keep them in sync, but that’s optional.
src/runtime/virtual/database.ts (1)
1-8: Database connection registry stub is fine; consider tightening typing laterThe
connectionConfigsregistry shape looks reasonable for a virtual module and being initialized as{}is appropriate for a stubbed runtime surface.If you want to reduce
anyleakage later, you could introduce a named type and/or replaceanywithunknownand narrow at use sites, e.g.:type ConnectionConfig = { connector: (options: unknown) => Connector; options: unknown; }; export const connectionConfigs: Record<string, ConnectionConfig> = {};Not required for this PR, but it can improve downstream type safety once the config shapes stabilise.
src/runtime/internal/virtual/server-assets.ts (1)
1-13: Deduplicate internal vs public server-asset stubs to avoid driftThis internal stub mirrors
src/runtime/virtual/server-assets.tsexactly (same three functions, same placeholder return values, noassetsexport).Given you’ll likely need to adjust the public stub to align with
#nitro/virtual/storage(e.g. by adding anassetsexport or changing behavior), consider:
- Re-exporting from the public module here, or
- Sharing a common implementation module that both internal and public virtuals import.
That way, fixes to the server-assets contract only need to be made in one place and internal/public surfaces stay in sync.
src/runtime/internal/virtual/feature-flags.ts (1)
1-7: Internal feature flags mirror public ones; consider sharing definitionsThis internal virtual correctly mirrors the public
#nitro/virtual/feature-flagssurface, so feature checks remain consistent.To reduce the risk of them drifting over time, you might later:
- Re-export the public flags here, or
- Move the constants into a shared module and import from both internal and public virtuals.
Not necessary for this PR, but it would simplify future maintenance.
src/runtime/internal/virtual/public-assets.ts (1)
1-15: Avoid duplicating identical public-assets stubs between internal and public modulesThis implementation is identical to
src/runtime/virtual/public-assets.ts. To reduce drift risk, consider having one module implement the stub (e.g., the public one) and have the other simply re-export from it:- import type { PublicAsset } from "nitro/types"; - - export const publicAssetBases: string[] = []; - export const isPublicAssetURL: (id: string) => boolean = () => false; - export const getPublicAssetMeta: ( - id: string -) => { maxAge?: number } | null = () => null; - export const readAsset: (id: string) => Promise<Buffer> = async () => { - throw new Error("Asset not found"); - }; - export const getAsset: (id: string) => PublicAsset | null = () => null; +export * from "../../virtual/public-assets";(or the inverse re-export direction, depending on which path you treat as canonical).
src/runtime/internal/runtime-config.ts (1)
23-27: Consider tightening EnvOptions defaults and env access for robustnessA few small robustness tweaks you might want to consider:
EnvOptions.altPrefixis optional but always concatenated (opts.altPrefix + envKey), which produces keys like"undefinedFOO_BAR"when it isn’t set. Harmless but noisy; you could default prefixes before use:-function getEnv(key: string, opts: EnvOptions) { - const envKey = snakeCase(key).toUpperCase(); - return ( - process.env[opts.prefix + envKey] ?? process.env[opts.altPrefix + envKey] - ); -} +function getEnv(key: string, opts: EnvOptions) { + const envKey = snakeCase(key).toUpperCase(); + const prefix = opts.prefix ?? ""; + const altPrefix = opts.altPrefix ?? ""; + return process.env[prefix + envKey] ?? process.env[altPrefix + envKey]; +}
getEnvand_expandFromEnvuseprocess.envdirectly, whilegetRuntimeConfigreads fromglobalThis.process?.env || {}. For consistency and to play nicer with non-Node targets, you could thread the env object throughEnvOptions(e.g.env?: NodeJS.ProcessEnv) and avoid unguardedprocessaccess inside helpers.These aren’t blockers but would make the env-application logic a bit more predictable and portable.
Also applies to: 57-59, 64-76
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (77)
package.json(1 hunks)src/build/plugins/resolve.ts(1 hunks)src/build/virtual/database.ts(1 hunks)src/build/virtual/error-handler.ts(1 hunks)src/build/virtual/feature-flags.ts(1 hunks)src/build/virtual/plugins.ts(1 hunks)src/build/virtual/public-assets.ts(7 hunks)src/build/virtual/renderer-template.ts(1 hunks)src/build/virtual/routing-meta.ts(1 hunks)src/build/virtual/routing.ts(2 hunks)src/build/virtual/runtime-config.ts(1 hunks)src/build/virtual/server-assets.ts(1 hunks)src/build/virtual/storage.ts(2 hunks)src/build/virtual/tasks.ts(1 hunks)src/build/virtual/types/feature-flags.d.ts(0 hunks)src/build/virtual/types/plugins.d.ts(0 hunks)src/build/virtual/types/public-assets.d.ts(0 hunks)src/build/virtual/types/renderer-template.d.ts(0 hunks)src/build/virtual/types/routing-meta.d.ts(0 hunks)src/build/virtual/types/routing.d.ts(0 hunks)src/build/virtual/types/runtime-config.d.ts(0 hunks)src/build/virtual/types/server-assets.d.ts(0 hunks)src/build/virtual/types/storage.d.ts(0 hunks)src/presets/_nitro/runtime/nitro-dev.ts(1 hunks)src/presets/_nitro/runtime/service-worker.ts(1 hunks)src/presets/bun/runtime/bun.ts(1 hunks)src/presets/cloudflare/entry-exports.ts(1 hunks)src/presets/cloudflare/runtime/_module-handler.ts(1 hunks)src/presets/cloudflare/runtime/cloudflare-durable.ts(1 hunks)src/presets/cloudflare/runtime/cloudflare-module.ts(1 hunks)src/presets/cloudflare/runtime/cloudflare-pages.ts(1 hunks)src/presets/deno/runtime/deno-deploy.ts(1 hunks)src/presets/deno/runtime/deno-server.ts(1 hunks)src/presets/netlify/runtime/netlify-edge.ts(1 hunks)src/presets/node/runtime/node-cluster.ts(1 hunks)src/presets/node/runtime/node-middleware.ts(1 hunks)src/presets/node/runtime/node-server.ts(1 hunks)src/runtime/internal/app.ts(2 hunks)src/runtime/internal/database.ts(1 hunks)src/runtime/internal/routes/dev-tasks.ts(1 hunks)src/runtime/internal/routes/openapi.ts(1 hunks)src/runtime/internal/routes/renderer-template.dev.ts(1 hunks)src/runtime/internal/routes/renderer-template.ts(1 hunks)src/runtime/internal/runtime-config.ts(2 hunks)src/runtime/internal/runtime-config.utils.ts(0 hunks)src/runtime/internal/static.ts(1 hunks)src/runtime/internal/storage.ts(1 hunks)src/runtime/internal/task.ts(1 hunks)src/runtime/internal/virtual/database.ts(1 hunks)src/runtime/internal/virtual/error-handler.ts(1 hunks)src/runtime/internal/virtual/feature-flags.ts(1 hunks)src/runtime/internal/virtual/plugins.d.ts(1 hunks)src/runtime/internal/virtual/public-assets.ts(1 hunks)src/runtime/internal/virtual/renderer-template.ts(1 hunks)src/runtime/internal/virtual/routing-meta.ts(1 hunks)src/runtime/internal/virtual/routing.ts(1 hunks)src/runtime/internal/virtual/runtime-config.ts(1 hunks)src/runtime/internal/virtual/server-assets.ts(1 hunks)src/runtime/internal/virtual/storage.ts(1 hunks)src/runtime/internal/vite/dev-entry.mjs(1 hunks)src/runtime/internal/vite/ssr-renderer.mjs(1 hunks)src/runtime/virtual/database.ts(1 hunks)src/runtime/virtual/error-handler.ts(1 hunks)src/runtime/virtual/feature-flags.ts(1 hunks)src/runtime/virtual/plugins.ts(1 hunks)src/runtime/virtual/polyfills.ts(1 hunks)src/runtime/virtual/public-assets.ts(1 hunks)src/runtime/virtual/renderer-template.ts(1 hunks)src/runtime/virtual/routing-meta.ts(1 hunks)src/runtime/virtual/routing.ts(1 hunks)src/runtime/virtual/runtime-config.ts(1 hunks)src/runtime/virtual/server-assets.ts(1 hunks)src/runtime/virtual/storage.ts(1 hunks)src/runtime/virtual/tasks.ts(1 hunks)test/unit/runtime-config.env.test.ts(1 hunks)test/unit/virtual.test.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (10)
- src/build/virtual/types/routing.d.ts
- src/build/virtual/types/routing-meta.d.ts
- src/build/virtual/types/public-assets.d.ts
- src/build/virtual/types/plugins.d.ts
- src/build/virtual/types/server-assets.d.ts
- src/build/virtual/types/renderer-template.d.ts
- src/runtime/internal/runtime-config.utils.ts
- src/build/virtual/types/storage.d.ts
- src/build/virtual/types/feature-flags.d.ts
- src/build/virtual/types/runtime-config.d.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-03T19:09:13.905Z
Learnt from: medz
Repo: nitrojs/nitro PR: 3834
File: src/presets/cloudflare/server-entry.ts:63-63
Timestamp: 2025-12-03T19:09:13.905Z
Learning: In the Nitro Cloudflare preset (src/presets/cloudflare/server-entry.ts), native errors from oxc-parser and Node.js readFile operations should be allowed to bubble up naturally without wrapping, as their native error messages are more user-friendly and provide better diagnostic information.
Applied to files:
src/presets/cloudflare/entry-exports.tssrc/runtime/virtual/error-handler.tssrc/presets/node/runtime/node-server.tssrc/presets/cloudflare/runtime/cloudflare-pages.tssrc/presets/cloudflare/runtime/cloudflare-durable.tssrc/runtime/internal/app.tssrc/build/virtual/error-handler.tssrc/presets/deno/runtime/deno-server.tssrc/presets/cloudflare/runtime/cloudflare-module.tssrc/presets/cloudflare/runtime/_module-handler.ts
🧬 Code graph analysis (17)
src/runtime/internal/virtual/plugins.d.ts (2)
src/build/virtual/plugins.ts (1)
plugins(4-24)src/types/runtime/nitro.ts (1)
NitroAppPlugin(12-18)
src/runtime/virtual/runtime-config.ts (1)
src/types/config.ts (1)
NitroRuntimeConfig(382-393)
src/runtime/internal/virtual/runtime-config.ts (2)
src/build/virtual/runtime-config.ts (1)
runtimeConfig(3-12)src/types/config.ts (1)
NitroRuntimeConfig(382-393)
src/build/plugins/resolve.ts (1)
src/runtime/meta.ts (1)
runtimeDir(9-9)
src/runtime/virtual/error-handler.ts (2)
src/types/handler.ts (1)
NitroErrorHandler(77-92)src/build/virtual/error-handler.ts (1)
errorHandler(5-42)
src/runtime/internal/virtual/renderer-template.ts (1)
src/build/virtual/renderer-template.ts (1)
rendererTemplate(9-53)
src/runtime/virtual/plugins.ts (1)
src/types/runtime/nitro.ts (1)
NitroAppPlugin(12-18)
src/runtime/internal/virtual/routing.ts (1)
src/types/route-rules.ts (1)
MatchedRouteRule(30-36)
src/runtime/virtual/routing-meta.ts (1)
src/types/handler.ts (1)
NitroRouteMeta(12-16)
src/runtime/internal/virtual/routing-meta.ts (1)
src/types/handler.ts (1)
NitroRouteMeta(12-16)
src/runtime/virtual/public-assets.ts (1)
src/runtime/virtual/server-assets.ts (1)
readAsset(3-5)
src/runtime/virtual/routing.ts (1)
src/types/route-rules.ts (1)
MatchedRouteRule(30-36)
src/runtime/internal/app.ts (2)
src/runtime/internal/vite/dev-entry.mjs (1)
nitroApp(8-8)src/types/runtime/nitro.ts (1)
NitroApp(5-10)
src/runtime/internal/virtual/server-assets.ts (1)
src/runtime/internal/virtual/public-assets.ts (1)
readAsset(11-13)
src/runtime/virtual/server-assets.ts (1)
src/runtime/virtual/public-assets.ts (1)
readAsset(11-13)
src/runtime/internal/virtual/public-assets.ts (1)
src/runtime/internal/virtual/server-assets.ts (1)
readAsset(3-5)
src/runtime/internal/virtual/error-handler.ts (1)
src/build/virtual/error-handler.ts (1)
errorHandler(5-42)
🪛 GitHub Check: tests-checks (ubuntu-latest)
tsconfig.json
[failure] 14-14:
Non-relative paths are not allowed when 'baseUrl' is not set. Did you forget a leading './'?
🪛 GitHub Check: tests-rollup (ubuntu-latest)
tsconfig.json
[failure] 14-14:
Non-relative paths are not allowed when 'baseUrl' is not set. Did you forget a leading './'?
⏰ 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). (1)
- GitHub Check: tests-rolldown (windows-latest)
🔇 Additional comments (50)
src/runtime/internal/static.ts (1)
10-14: Import path migration to#nitro/virtual/public-assetslooks correctThe updated import source matches the new public virtual namespace and is consistent with the rest of the handler’s usage; no behavioral change introduced here.
src/runtime/internal/task.ts (1)
10-10: Tasks virtual import refactor is consistentSwitching to
#nitro/virtual/taskskeeps the existingtasks/scheduledTasksusage intact and aligns with the new virtual namespace; no runtime behavior change here.src/runtime/internal/vite/ssr-renderer.mjs (1)
1-5: Vite runtime import swap preserves behaviorUsing
nitro/vite/runtimeforfetchViteEnvkeeps the SSR renderer logic identical while routing through the new subpath export; looks good.src/build/virtual/routing-meta.ts (1)
5-5: Routing-meta virtual id migration verifiedThe virtual module id has been successfully updated to
#nitro/virtual/routing-meta. Verification confirms no remaining references to the old id exist, and the import insrc/runtime/internal/routes/openapi.tsalready uses the new id correctly.src/runtime/virtual/routing-meta.ts (1)
1-7: Typed routing metadata container looks good
handlersMetais a minimal, well‑typed placeholder for per‑route metadata, and the shape matchesNitroRouteMetausage. No issues from a typing or runtime perspective.src/presets/_nitro/runtime/service-worker.ts (1)
3-3: UpdatedisPublicAssetURLimport is consistent with new virtual namespaceSwitching the import to
#nitro/virtual/public-assetsaligns with the new#nitro/virtual/*mapping and the rest of the PR. No behavioral changes here.src/presets/netlify/runtime/netlify-edge.ts (1)
3-3: Netlify EdgeisPublicAssetURLimport correctly migrated to#nitro/virtualThe new
#nitro/virtual/public-assetsimport matches the updated virtual module namespace and should resolve via the new import map without affecting behavior.package.json (1)
27-33: New exports/imports wiring is consistent with the virtual/runtime refactorAdding
./vite/runtimeand mapping#nitro/runtime/*/#nitro/virtual/*intodist/runtimealigns with the updated import paths elsewhere in the PR. No lingering references to old#runtime/*or#vite-runtimespecifiers remain in the codebase, confirming the migration is complete.src/build/virtual/renderer-template.ts (1)
11-11: Renderer virtual ID aligned with new#nitro/virtual/*namespaceThe
idnow matches the new public renderer virtual import path and stays consistent with the runtime imports; no behavioral change. Looks good.src/presets/cloudflare/runtime/_module-handler.ts (1)
6-6: Cloudflare cron tasks now sourced from#nitro/runtime/taskImport path migration to
#nitro/runtime/taskmatches the new runtime namespace while preserving the existingrunCronTasksusage. No issues from this change alone.src/build/virtual/error-handler.ts (1)
7-7: Error handler virtual ID updated to public#nitro/virtualpathThe new
idstring matches the#nitro/virtual/error-handlernamespace and doesn’t affect generated error-handling logic. Change is consistent and safe.src/build/virtual/routing.ts (1)
12-12: Routing virtual ID and route-rules import aligned with new Nitro namespacesThe routing virtual
idand__routeRules__import now use#nitro/virtual/routingand#nitro/runtime/route-rules, matching the updated public surface without altering routing behavior.Also applies to: 26-26
src/runtime/internal/database.ts (1)
3-3: Database connection config now sourced from#nitro/virtual/databaseSwitching
connectionConfigsto the new#nitro/virtual/databasemodule keepsuseDatabasesemantics intact while making the virtual resolvable at runtime for tests. Looks consistent with the PR goal.src/build/virtual/tasks.ts (1)
6-6: Tasks virtual ID migrated to#nitro/virtual/tasksThe updated
idstring is consistent with the new virtual namespace; scheduled-task and task-resolution logic remain the same.src/runtime/internal/routes/renderer-template.dev.ts (1)
7-7: Dev renderer now imports from#nitro/virtual/renderer-templateImport path update matches the new public virtual entry while keeping the dev HTML rendering flow the same. No additional concerns.
src/runtime/virtual/plugins.ts (1)
1-3: New runtime stub for#nitro/virtual/pluginslooks correctThe stub exports a typed
pluginsarray defaulting to empty, giving Node a concrete module to resolve while still allowing build-time injection. This matches the NitroAppPlugin type and should be safe.src/runtime/internal/routes/dev-tasks.ts (1)
4-4: LGTM! Import path updated to public virtual module.The import path has been correctly updated from the internal virtual path to the new public Nitro virtual path, aligning with the PR's objective to make virtual modules resolvable outside the bundle.
test/unit/virtual.test.ts (1)
1-11: LGTM! Appropriate test for virtual module resolution.The test correctly validates that the virtual module system works by verifying the serverFetch function can be called and returns expected 404 behavior. This aligns with the PR's goal of making virtual modules resolvable outside the bundle context.
src/runtime/internal/storage.ts (1)
3-3: LGTM! Import path updated to public virtual module.The import path has been correctly updated to use the new public Nitro virtual path, maintaining the same functionality while improving module resolvability.
src/build/virtual/plugins.ts (1)
6-6: LGTM! Virtual module ID updated to public path.The virtual module identifier has been correctly updated to use the new public Nitro virtual namespace, aligning with the PR's refactoring objectives.
src/runtime/internal/routes/openapi.ts (1)
13-13: LGTM! Import path updated to public virtual module.The import path has been correctly updated to use the new public Nitro virtual path, consistent with the PR's migration strategy.
src/runtime/internal/vite/dev-entry.mjs (1)
5-6: LGTM! Import paths updated to public namespaces.Both import paths have been correctly updated:
- Runtime app import now uses "#nitro/runtime/app"
- Virtual feature flags import now uses "#nitro/virtual/feature-flags"
These changes align with the PR's objective to standardize virtual module paths.
src/runtime/internal/routes/renderer-template.ts (1)
2-2: LGTM! Import path updated to public virtual module.The import path has been correctly updated to use the new public Nitro virtual path, maintaining functionality while improving module resolvability outside the bundle context.
src/runtime/virtual/storage.ts (1)
1-5: StubinitStoragelooks correct for runtime-only virtualType-only import plus a throwing implementation is a sensible pattern here: callers get proper
Storagetyping while misuse outside Nitro runtime fails fast with a clear message. No issues from a runtime or typing perspective.src/presets/node/runtime/node-cluster.ts (1)
7-10: Runtime/virtual import rewrites are consistentImports have been correctly migrated to
#nitro/runtime/*and#nitro/virtual/feature-flagswith unchanged usage, so behavior should remain identical while aligning with the new public subpath scheme.src/build/virtual/feature-flags.ts (1)
5-5: Virtual module id now matches new runtime import pathSwitching the
idto"#nitro/virtual/feature-flags"aligns this virtual with the updated preset imports and keeps the generated feature flag exports unchanged.src/presets/deno/runtime/deno-deploy.ts (1)
7-8: Deno deploy preset correctly targets new runtime/virtual pathsThe imports for
resolveWebsocketHooksandhasWebSocketnow match the new#nitro/runtime/*and#nitro/virtual/*namespaces, with unchanged WebSocket wiring logic.src/presets/node/runtime/node-middleware.ts (1)
6-8: Node middleware imports updated consistentlyThe middleware now pulls
startScheduleRunner,resolveWebsocketHooks, andhasWebSocketfrom the new#nitro/runtime/*and#nitro/virtual/*locations, matching the virtual id change and other presets, with no behavioral change.src/presets/deno/runtime/deno-server.ts (1)
7-10: Deno server runtime wiring matches new namespacesThe Deno preset now sources task runner, error trapping, WebSocket hooks, and feature flags from the
#nitro/runtime/*and#nitro/virtual/*paths, keeping existing behavior while aligning with the new public subpaths.test/unit/runtime-config.env.test.ts (1)
3-3: Test import follows runtime-config refactorUpdating
applyEnvto import fromruntime-config.tsmatches the internal refactor while keeping the test cases intact; any mismatch in exports will be caught by the test/TS pipeline.src/presets/cloudflare/runtime/cloudflare-module.ts (1)
5-8: Cloudflare preset imports aligned with new runtime/virtual modulesThe Cloudflare runtime now consumes
isPublicAssetURL,resolveWebsocketHooks, andhasWebSocketfrom the new#nitro/virtual/*and#nitro/runtime/*entry points, preserving existing asset and WebSocket behavior with the updated subpath layout.src/presets/bun/runtime/bun.ts (1)
7-10: LGTM! Import path updates are correct.The import paths have been successfully migrated to the new public Nitro virtual and runtime paths, aligning with the PR's objective to standardize module aliases.
src/runtime/internal/virtual/runtime-config.ts (1)
1-6: LGTM! Stub implementation is appropriate.The empty stub implementation for
runtimeConfigcorrectly provides the expected type structure while allowing the runtime to operate outside of a bundle, which aligns with the PR's testing objectives.src/runtime/internal/virtual/storage.ts (1)
3-5: LGTM! Error-throwing stub is appropriate.The stub implementation correctly guards runtime-only functionality with a clear error message, preventing misuse outside the Nitro runtime context.
src/presets/cloudflare/entry-exports.ts (1)
13-14: LGTM! Virtual entry path updated correctly.The virtual entry ID has been successfully migrated to the new public Nitro virtual path, consistent with the broader refactoring in this PR.
src/presets/cloudflare/runtime/cloudflare-durable.ts (1)
8-10: LGTM! Import paths migrated correctly.All three imports have been successfully updated to the new public Nitro virtual and runtime paths, maintaining consistency with the PR's refactoring objectives.
src/build/virtual/server-assets.ts (1)
20-20: LGTM! Virtual module ID updated correctly.The server assets virtual module ID has been successfully migrated to the new public Nitro virtual path.
src/build/virtual/database.ts (1)
7-7: LGTM! Virtual module ID updated correctly.The database virtual module ID has been successfully migrated to the new public Nitro virtual path.
src/runtime/internal/app.ts (2)
15-16: LGTM! Import paths migrated correctly.All virtual module imports have been successfully updated to the new public Nitro virtual paths, maintaining consistency with the PR's refactoring objectives.
Also applies to: 22-22, 30-30
90-92: The type assertion is safe and does not require additional verification or runtime checks.The
hasHooksflag is logically guaranteed to be true wheneverhasPluginsis true due to its build-time definition:hasHooks: nitro.options.features?.runtimeHooks ?? nitro.options.plugins.length > 0. When plugins exist (hasPluginsis true),hasHooksdefaults to true, ensuring the type assertion's assumption holds.src/runtime/internal/virtual/routing-meta.ts (1)
1-7: Typed routing metadata stub looks correctThe
handlersMetaexport has an appropriate shape for per-route metadata and an empty-array default that is safe for runtime and tests; no changes needed here.src/runtime/virtual/runtime-config.ts (1)
1-6: runtimeConfig stub aligns with NitroRuntimeConfigProviding a concrete
runtimeConfigwithapp: {}andnitro: {}gives a valid, mutable default for the#nitro/virtual/runtime-configentrypoint and supports non-bundled/testing scenarios without breaking the typed contract.src/presets/_nitro/runtime/nitro-dev.ts (1)
9-12: Dev worker imports correctly migrated to #nitro runtime/virtual pathsThe four imports now target
#nitro/runtime/*and#nitro/virtual/feature-flags, matching the new public namespaces while preserving existing scheduling, error trapping, and WebSocket behavior.src/build/virtual/public-assets.ts (1)
21-182: Public assets virtual ids/imports consistently moved to#nitro/virtual/*All virtual template ids and the generated imports (
public-assets,*-node,*-deno,*-null,*-inline, andpublic-assets-data) are now consistently under#nitro/virtual/..., and thereadAssetImportconstruction matches those ids, so behavior remains the same with updated resolution.src/build/virtual/runtime-config.ts (1)
5-10: runtime-config virtual id now matches public virtual entrypointSwitching the id to
#nitro/virtual/runtime-configcorrectly aligns this build-time virtual with the new public stub/module while keeping the generated payload unchanged.src/presets/cloudflare/runtime/cloudflare-pages.ts (1)
11-14: Cloudflare runtime now uses public #nitro runtime/virtual modulesThe updated imports for
isPublicAssetURL,runCronTasks,resolveWebsocketHooks, andhasWebSocketcleanly switch to the#nitro/runtime/*and#nitro/virtual/*namespaces without altering request, asset, WebSocket, or cron behavior; error propagation remains unchanged, which is desirable for diagnostics. Based on learnings, this keeps the preset’s error semantics consistent.src/runtime/internal/virtual/database.ts (1)
3-8: connectionConfigs stub provides a safe, usable defaultExporting
connectionConfigsas an initialized empty object with the expected{ connector, options }shape makes the internal virtual database module importable at runtime while remaining safe for code that iterates or augments the map.src/presets/node/runtime/node-server.ts (1)
6-9: Node server imports updated to new #nitro runtime/virtual namespacesThe Node preset correctly switches scheduling, error trapping, WebSocket resolution, and feature-flag imports to
#nitro/runtime/*and#nitro/virtual/feature-flags, preserving existing behavior with the new module resolution scheme.src/runtime/virtual/renderer-template.ts (1)
1-7: Renderer template stub is clear and safeThe renderer template virtual cleanly exposes:
- A callable
rendererTemplatethat always returns a clear placeholder string.- Dev-only metadata (
rendererTemplateFile,isStaticTemplate) defaulted toundefined.This gives callers a stable API while making it obvious when no real renderer template is wired. No issues from what’s shown.
src/runtime/internal/virtual/public-assets.ts (1)
3-15: Stubbed internal public-assets API is coherent and safe for nowReturning empty bases,
false/null, and throwing onreadAssetgives deterministic failure modes, which is reasonable for a virtual stub that’s only meant to be overridden/mocked at build or test time. No functional issues from this implementation itself.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/runtime/virtual/runtime-config.ts (1)
4-7: Consider minimal safe defaults for thenitrostub shapeRight now the stub sets
nitro: {}. If any consumers assumeruntimeConfig.nitro.routeRules(or similar) is at least an object, this can turn a misconfiguration into a hard runtime error instead of just a no‑op.To keep the stub harmless while still empty, consider something like:
-export const runtimeConfig: NitroRuntimeConfig = { - app: {}, - nitro: {}, -}; +export const runtimeConfig: NitroRuntimeConfig = { + app: {}, + nitro: { + routeRules: {}, + }, +};This preserves the “stub” behavior but avoids
Cannot read properties of undefinedwhen the virtual is accidentally used at runtime. Please double‑check current call sites ofruntimeConfig.nitro/routeRulesto confirm whether this is needed.src/runtime/virtual/routing.ts (1)
7-28: Routing stub behavior is clear; consider documenting its contractThe four exports (
findRoute,findRouteRules,globalMiddleware,findRoutedMiddleware) all behave as safe no‑ops (undefined/empty arrays), which is appropriate for a stub used when no Nitro builder/plugin is present.To avoid confusion later and keep this in sync with the real routing implementation, consider adding a brief module‑level comment that this file is a stub that’s replaced by the built runtime version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/runtime/virtual/_runtime_warn.ts(1 hunks)src/runtime/virtual/database.ts(1 hunks)src/runtime/virtual/error-handler.ts(1 hunks)src/runtime/virtual/feature-flags.ts(1 hunks)src/runtime/virtual/plugins.ts(1 hunks)src/runtime/virtual/polyfills.ts(1 hunks)src/runtime/virtual/public-assets.ts(1 hunks)src/runtime/virtual/renderer-template.ts(1 hunks)src/runtime/virtual/routing-meta.ts(1 hunks)src/runtime/virtual/routing.ts(1 hunks)src/runtime/virtual/runtime-config.ts(1 hunks)src/runtime/virtual/server-assets.ts(1 hunks)src/runtime/virtual/storage.ts(1 hunks)src/runtime/virtual/tasks.ts(1 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- src/runtime/virtual/feature-flags.ts
- src/runtime/virtual/storage.ts
- src/runtime/virtual/error-handler.ts
- src/runtime/virtual/database.ts
- src/runtime/virtual/plugins.ts
- src/runtime/virtual/renderer-template.ts
- src/runtime/virtual/server-assets.ts
- src/runtime/virtual/tasks.ts
- src/runtime/virtual/public-assets.ts
- src/runtime/virtual/routing-meta.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/runtime/virtual/routing.ts (1)
src/types/route-rules.ts (1)
MatchedRouteRule(30-36)
src/runtime/virtual/runtime-config.ts (1)
src/types/config.ts (1)
NitroRuntimeConfig(382-393)
⏰ 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). (3)
- GitHub Check: tests-rollup (ubuntu-latest)
- GitHub Check: tests-rolldown (windows-latest)
- GitHub Check: tests-rollup (windows-latest)
🔇 Additional comments (5)
src/runtime/virtual/runtime-config.ts (1)
1-2: Side‑effect import for runtime warning is appropriateImporting
"./_runtime_warn.ts"purely for its side effects cleanly ensures a warning is emitted whenever this stub is used, without coupling callers to the warning mechanism. No issues here.src/runtime/virtual/polyfills.ts (1)
1-3: Stub polyfills module with warning hook looks goodUsing a side‑effect import for the warning plus an empty default export is a clean way to provide a no‑op polyfills module when running without a builder/plugin.
tsconfig.json (1)
12-12: Type configuration is consistent with the rest of the setupKeeping
"types": ["node", "@cloudflare/workers-types"]and removing the internal virtual paths alias (per the broader PR) makes sense and should avoid the previous path‑mapping error. Nothing else to change here; just ensuretsc/your type‑check still pass on CI after this adjustment.src/runtime/virtual/_runtime_warn.ts (1)
1-8: Runtime warning helper is concise and behaves as intendedGating the
consola.warncall behind!isTestensures tests stay quiet while still surfacing misconfigured runtime imports the first time any virtual module is loaded. Import‑time side effects are appropriate here given the goal.src/runtime/virtual/routing.ts (1)
1-6: Type‑only imports keep the routing stub lightweightImporting the routing/types as
import typeand using a side‑effect import for the warning strikes a good balance: no runtime dependency on h3/rou3 while still exposing the correct surface area and emitting the misconfiguration warning.
| "#runtime/*": "./dist/runtime/internal/*.mjs", | ||
| "#vite-runtime": "./dist/runtime/vite-runtime.mjs" | ||
| "#nitro/runtime/*": "./dist/runtime/internal/*.mjs", | ||
| "#nitro/virtual/*": "./dist/runtime/virtual/*.mjs" |
There was a problem hiding this comment.
Very clean structure. ❤️
Nitro runtime depends on generated virtual modules that only work during the build phase.
This makes unit testing and writing shared code difficult when project code imports
nitro/runtime,nitro/storage, etc (even if unused, unit testing needs manual mocks or a custom nitro env and shared code needs to be splitted)This PR introduces stub implementations for all internal virtual modules using Node.js sub-path imports (
#nitro/virtual/*used internally). This allows the runtime imports to work outside of the Nitro bundle.Outside of tests, a warning will be shown: