fix(vite-plugin-nitro): update nitro to 3.0.260415-beta#2300
Conversation
Remove workarounds for OXC tsconfig resolution, Vercel build config validation, and Vercel function config that are now handled by Nitro itself. Remove moduleSideEffects for zone.js. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
scope:repo
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRemoved Vercel-specific build-output checks and Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Key observations for review
🚥 Pre-merge checks | ✅ 2✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin-nitro/src/lib/build-server.ts (1)
19-106:⚠️ Potential issue | 🔴 CriticalTest regression:
build-server.spec.tsexpects a Vercel validation error that is no longer thrown bybuildServer().This test was added in commit
07504a4calongside removal ofassertValidVercelBuildConfig. The commit message states validation is "now handled by Nitro itself," but the test still expectsbuildServer()to reject with the error messageNitro generated an empty Vercel build output config at "...".The current implementation has no validation—it simply awaits
build(nitro)and thenawait nitro.close(). The mockedbuild()writes an empty config file, but nothing validates or throws on that condition. Whenpnpm testruns, this test will fail because the expected error is never thrown.Either delete this test (if Nitro's own build flow now validates upstream) or rewrite it to assert against Nitro's error if the validation was intentionally delegated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-nitro/src/lib/build-server.ts` around lines 19 - 106, The test regression is caused because buildServer (function buildServer) no longer performs Vercel config validation and so the spec expecting the Vercel-specific error fails; fix by updating the test (build-server.spec.ts) to either remove the old expectation for "Nitro generated an empty Vercel build output config at ..." or rewrite the assertion to match the new behavior — specifically, have the test stub/mock build() to throw or reject and assert that buildServer rejects with that Nitro/build error (or assert that build() was invoked and nitro.close() is reached/fails as appropriate); alternatively, if you intentionally want to preserve the original validation inside buildServer, reintroduce a validation step after await build(nitro) that checks the generated Vercel config and throws the original message to satisfy the existing test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/vite-plugin-nitro/src/lib/build-server.ts`:
- Around line 19-106: The test regression is caused because buildServer
(function buildServer) no longer performs Vercel config validation and so the
spec expecting the Vercel-specific error fails; fix by updating the test
(build-server.spec.ts) to either remove the old expectation for "Nitro generated
an empty Vercel build output config at ..." or rewrite the assertion to match
the new behavior — specifically, have the test stub/mock build() to throw or
reject and assert that buildServer rejects with that Nitro/build error (or
assert that build() was invoked and nitro.close() is reached/fails as
appropriate); alternatively, if you intentionally want to preserve the original
validation inside buildServer, reintroduce a validation step after await
build(nitro) that checks the generated Vercel config and throws the original
message to satisfy the existing test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a08e766e-4c85-45be-8e77-9dc73a2e944e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (3)
packages/vite-plugin-nitro/src/lib/build-server.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
…alidation Replace the test for assertValidVercelBuildConfig (now handled by Nitro upstream) with a test verifying the rollup builder default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Pass test.css: true in JIT plugin tests so preprocessCSS is called - Add createPrinter and createSourceFile to TS mock in transform test - Align i18n tests with current implementation (plain string translations, remove tests for unimplemented functions) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
scope:router
|
This PR touches multiple package scopes: Please confirm the changes are closely related. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/router/src/lib/i18n/provide-i18n.spec.ts (1)
1-287:⚠️ Potential issue | 🔴 CriticalCritical: Broken i18n registry API — platform plugin cannot build.
The platform plugin at
packages/platform/src/lib/i18n-component-registry-plugin.ts(line 44) importsɵregisterI18nComponentDeffrom@analogjs/router, but this symbol is not exported anywhere in the router package. A search ofpackages/router/srcfound zero occurrences of any of the four registry helpers (ɵregisterI18nComponentDef,ɵresetI18nComponentDefCache,getI18nComponentDefRegistrySize,clearI18nComponentDefRegistry).This breaks the SSR i18n build for any app that enables the
i18noption in the platform plugin configuration. Either:
- Restore the registry exports and update the test suite with minimal smoke tests, or
- Remove the i18n-component-registry-plugin and document the removal as a breaking change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router/src/lib/i18n/provide-i18n.spec.ts` around lines 1 - 287, The platform plugin is importing missing registry APIs (ɵregisterI18nComponentDef, ɵresetI18nComponentDefCache, getI18nComponentDefRegistrySize, clearI18nComponentDefRegistry) that are not exported from the router package; restore these exports from the router (implement or re-expose them from the i18n module such as provide-i18n) and update the test suite (provide-i18n.spec.ts) to include minimal smoke tests that import and call each registry helper to ensure the symbols are exported and callable; ensure the exported function names exactly match ɵregisterI18nComponentDef, ɵresetI18nComponentDefCache, getI18nComponentDefRegistrySize, and clearI18nComponentDefRegistry so the platform plugin build succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/router/src/lib/i18n/provide-i18n.spec.ts`:
- Around line 1-287: The platform plugin is importing missing registry APIs
(ɵregisterI18nComponentDef, ɵresetI18nComponentDefCache,
getI18nComponentDefRegistrySize, clearI18nComponentDefRegistry) that are not
exported from the router package; restore these exports from the router
(implement or re-expose them from the i18n module such as provide-i18n) and
update the test suite (provide-i18n.spec.ts) to include minimal smoke tests that
import and call each registry helper to ensure the symbols are exported and
callable; ensure the exported function names exactly match
ɵregisterI18nComponentDef, ɵresetI18nComponentDefCache,
getI18nComponentDefRegistrySize, and clearI18nComponentDefRegistry so the
platform plugin build succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ade7b42-7332-4c68-b1fb-018c4574aaf3
📒 Files selected for processing (4)
packages/router/src/lib/i18n/provide-i18n.spec.tspackages/vite-plugin-angular/src/lib/angular-jit-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin-transform.spec.tspackages/vite-plugin-nitro/src/lib/build-server.spec.ts
✅ Files skipped from review due to trivial changes (1)
- packages/vite-plugin-angular/src/lib/angular-jit-plugin.spec.ts
The beta merge (9167e96) brought in the updated i18n tests but kept alpha's simpler provide-i18n.ts, causing 14 test failures. Restore the full beta implementation: - loadTranslationsRuntime uses @angular/localize's loadTranslations - clearTranslationsRuntime clears translations between SSR requests - initI18n clears before loading to prevent locale mixing - Component def registry (ɵregisterI18nComponentDef, ɵresetI18nComponentDefCache) for resetting cached tViews between SSR renders - i18nComponentRegistryPlugin wired into platform plugin - render.ts resets tView cache before each render - Add @angular/localize dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
PR Checklist
Closes #
Affected scope
Recommended merge strategy for maintainer [optional]
Commit preservation note [optional]
What is the new behavior?
Updates Nitro from
3.0.260311-betato3.0.260415-beta. Removes workarounds that are now handled upstream by Nitro:ensureSsrTsconfig)assertValidVercelBuildConfig)ensureVercelFunctionConfig)moduleSideEffectsforzone.jsTest plan
nx build analog-apppnpm buildpnpm testDoes this PR introduce a breaking change?
Other information
🤖 Generated with Claude Code