fix: add v0 migration warnings and docs for breaking config changes - #679
Merged
Conversation
Detect and auto-migrate `reverseProxyIntercept` to `proxy` in registry config with a build warning. Document the rename and Google Maps component consolidation in the v0-to-v1 migration guide.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigration docs and examples updated to replace boolean registry enablement ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
packages/script/src/normalize.ts (1)
43-60: Consider deduplicating the warning message construction.The same warning template is repeated across branches; extracting a tiny helper will reduce drift risk.
♻️ Suggested cleanup
export function migrateDeprecatedRegistryKeys( registry: Record<string, unknown>, warn: (msg: string) => void, ): void { + const warnRename = (registryKey: string, from: string) => + warn(`registry.${registryKey}: \`${from}\` has been renamed to \`proxy\`. Please update your config. Auto-migrating for now.`) + for (const key of Object.keys(registry)) { @@ const opts = entry[1] if (opts && typeof opts === 'object' && 'reverseProxyIntercept' in opts) { - warn(`registry.${key}: \`reverseProxyIntercept\` has been renamed to \`proxy\`. Please update your config. Auto-migrating for now.`) + warnRename(key, 'reverseProxyIntercept') const o = opts as Record<string, unknown> @@ if ('reverseProxyIntercept' in obj) { - warn(`registry.${key}: \`reverseProxyIntercept\` has been renamed to \`proxy\`. Please update your config. Auto-migrating for now.`) + warnRename(key, 'reverseProxyIntercept') obj.proxy ??= obj.reverseProxyIntercept delete obj.reverseProxyIntercept } @@ if (so && typeof so === 'object' && 'reverseProxyIntercept' in so) { - warn(`registry.${key}: \`scriptOptions.reverseProxyIntercept\` has been renamed to \`proxy\`. Please update your config. Auto-migrating for now.`) + warnRename(key, 'scriptOptions.reverseProxyIntercept') const s = so as Record<string, unknown>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/normalize.ts` around lines 43 - 60, The warning text is duplicated; extract a small helper (e.g., warnRenamed or warnFieldRename) and replace the repeated warn(...) calls in normalize.ts so both top-level and nested cases call that helper with the registry key and field path; keep the existing message format including registry.${key}, the oldName (reverseProxyIntercept), newName (proxy), and "Auto-migrating for now.", and do not change the subsequent migration logic that sets proxy ??= reverseProxyIntercept and deletes reverseProxyIntercept in the branches that reference opts, obj, and obj.scriptOptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/script/src/normalize.ts`:
- Around line 43-60: The warning text is duplicated; extract a small helper
(e.g., warnRenamed or warnFieldRename) and replace the repeated warn(...) calls
in normalize.ts so both top-level and nested cases call that helper with the
registry key and field path; keep the existing message format including
registry.${key}, the oldName (reverseProxyIntercept), newName (proxy), and
"Auto-migrating for now.", and do not change the subsequent migration logic that
sets proxy ??= reverseProxyIntercept and deletes reverseProxyIntercept in the
branches that reference opts, obj, and obj.scriptOptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81ee404c-3c78-4b50-bece-5ce12b03f46b
📒 Files selected for processing (4)
docs/content/docs/4.migration-guide/1.v0-to-v1.mdpackages/script/src/module.tspackages/script/src/normalize.tstest/unit/normalize.test.ts
In v0, all configured scripts auto-loaded. In v1, a trigger is required for auto-loading. Emit a build warning when input fields are provided without a trigger so v0 users know their scripts stopped loading. Also expand the migration guide with a dedicated section explaining this behavior change with before/after examples.
…rigger: false`
- `true` as a registry value now emits a deprecation warning; use
`{ trigger: 'onNuxtReady' }` instead
- `trigger: false` is a valid explicit opt-out (infrastructure only)
- Missing trigger warning now checks key presence, not truthiness
- Update all docs to use explicit `{ trigger: 'onNuxtReady' }` instead of `true`
- Update scripts.nuxt.com code gen to emit trigger in all generated configs
…false positives - Actually call migrateDeprecatedRegistryKeys() before normalization (was dead code) - Add [nuxt-scripts] prefix to all warnings in normalize.ts for consistency - Filter env-var-only defaults from trigger warning to avoid false positives - Clarify trigger: false in migration guide (proxy routes, types, bundling only) - Condense config migration table (details already covered in prose above)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/content/docs/4.migration-guide/1.v0-to-v1.md (1)
223-223: Use active voice to satisfy lint style warning.“...no
<script>tag is injected” can be rewritten in active voice.Suggested wording
-If you only need infrastructure without loading the script on the page, set `trigger: false` explicitly. This registers proxy routes, TypeScript types, and bundling config, but no `<script>`{lang="html"} tag is injected. Useful when you load the script yourself via a component or composable. +If you only need infrastructure without loading the script on the page, set `trigger: false` explicitly. This registers proxy routes, TypeScript types, and bundling config, but Nuxt does not inject a `<script>`{lang="html"} tag. Useful when you load the script yourself via a component or composable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/4.migration-guide/1.v0-to-v1.md` at line 223, The sentence uses passive voice ("no <script> tag is injected"); change it to active voice by rephrasing it to explicitly name the actor and action—e.g., replace "but no `<script>`{lang=\"html\"} tag is injected." with "but the integration does not inject a `<script>`{lang=\"html`"} tag." Ensure the revised sentence appears in the same paragraph that begins "If you only need infrastructure without loading the script on the page, set `trigger: false` explicitly." and keep surrounding wording about proxy routes, TypeScript types, and bundling config unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/4.migration-guide/1.v0-to-v1.md`:
- Line 243: Update the migration note that currently implies `bundle` was
replaced by `trigger`: split the single sentence into two clear points — first
state that tuple/boolean shapes (e.g. `true` shorthand and array tuple syntax)
were replaced by a flat config shape (referencing the `{ id: '...' }` → `{ id:
'...', trigger: 'onNuxtReady' }` example), and second state that auto-loading
now requires an explicit `trigger` option; explicitly mention that `bundle`
remains a supported top-level flat option and is not replaced by `trigger` so
readers are not misled.
---
Nitpick comments:
In `@docs/content/docs/4.migration-guide/1.v0-to-v1.md`:
- Line 223: The sentence uses passive voice ("no <script> tag is injected");
change it to active voice by rephrasing it to explicitly name the actor and
action—e.g., replace "but no `<script>`{lang=\"html\"} tag is injected." with
"but the integration does not inject a `<script>`{lang=\"html`"} tag." Ensure
the revised sentence appears in the same paragraph that begins "If you only need
infrastructure without loading the script on the page, set `trigger: false`
explicitly." and keep surrounding wording about proxy routes, TypeScript types,
and bundling config unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6fbc6b9-5461-4dae-88b6-a64a8bc6f62e
📒 Files selected for processing (3)
docs/content/docs/4.migration-guide/1.v0-to-v1.mdpackages/script/src/module.tspackages/script/src/normalize.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/script/src/module.ts
- packages/script/src/normalize.ts
| | `[{ id: '...' }, { bundle: true }]` | `{ id: '...' }` | Bundling is auto-enabled via capabilities; no need to opt in | | ||
| | `[{ id: '...' }, { trigger: 'onNuxtReady' }]` | `{ id: '...', trigger: 'onNuxtReady' }` | Array tuple syntax still works, but flat config is preferred | | ||
| | `googleAnalytics: 'mock'` | `googleAnalytics: 'mock'` | Unchanged; creates a stub for testing | | ||
| | `{ id: '...' }` | `{ id: '...', trigger: 'onNuxtReady' }` | Add an explicit `trigger` to auto-load. `true` shorthand, `bundle` option, and array tuple syntax are also replaced by flat config with `trigger`. | |
There was a problem hiding this comment.
Clarify bundle wording to avoid implying it was replaced by trigger.
Line 243 currently reads as if bundle was replaced by trigger, but bundle is still a supported flat top-level option (as shown later in this doc). Please split this into two points: (1) tuple/boolean shapes were replaced by flat config, and (2) auto-loading now requires trigger.
Suggested wording
-| `{ id: '...' }` | `{ id: '...', trigger: 'onNuxtReady' }` | Add an explicit `trigger` to auto-load. `true` shorthand, `bundle` option, and array tuple syntax are also replaced by flat config with `trigger`. |
+| `{ id: '...' }` | `{ id: '...', trigger: 'onNuxtReady' }` | Add an explicit `trigger` to auto-load. `true` shorthand and array tuple syntax are replaced by flat config. `bundle` remains supported as a flat top-level option when needed. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `{ id: '...' }` | `{ id: '...', trigger: 'onNuxtReady' }` | Add an explicit `trigger` to auto-load. `true` shorthand, `bundle` option, and array tuple syntax are also replaced by flat config with `trigger`. | | |
| | `{ id: '...' }` | `{ id: '...', trigger: 'onNuxtReady' }` | Add an explicit `trigger` to auto-load. `true` shorthand and array tuple syntax are replaced by flat config. `bundle` remains supported as a flat top-level option when needed. | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/docs/4.migration-guide/1.v0-to-v1.md` at line 243, Update the
migration note that currently implies `bundle` was replaced by `trigger`: split
the single sentence into two clear points — first state that tuple/boolean
shapes (e.g. `true` shorthand and array tuple syntax) were replaced by a flat
config shape (referencing the `{ id: '...' }` → `{ id: '...', trigger:
'onNuxtReady' }` example), and second state that auto-loading now requires an
explicit `trigger` option; explicitly mention that `bundle` remains a supported
top-level flat option and is not replaced by `trigger` so readers are not
misled.
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Related to #594
❓ Type of change
📚 Description
v0 users upgrading to v1 hit silent failures when using renamed or removed config keys. This PR adds build time detection and auto migration for
reverseProxyIntercept→proxy, warns on missing triggers, and documents the changes in the migration guide.Config migration (
normalize.ts,module.ts):migrateDeprecatedRegistryKeys()runs before normalization, detectingreverseProxyInterceptin flat objects, nestedscriptOptions, and array tuples. Auto rewrites toproxy(without clobbering existing values) and emits a[nuxt-scripts]prefixed warning.trueshorthand emits a deprecation warning pointing to{ trigger: 'onNuxtReady' }trigger, filtering out env-var-only defaults to avoid false positivestrigger: falsesupported as explicit infrastructure-only opt-out (proxy routes, types, bundling registered; no<script>tag injected)migrateDeprecatedRegistryKeys, additional tests fortruedeprecation andtrigger: falseDocs updates:
truereplaced with{ trigger: 'onNuxtReady' }across all registry script docstrigger: falseclarified: registers infrastructure only, useful when loading via component/composableMigration guide (
v0-to-v1.md):reverseProxyIntercept→proxyrename documentation