feat: env-var overrides for scripts.globals (single-build, multi-deploy)#780
Conversation
Globals input is now exposed via runtimeConfig.public.scriptsGlobals and merged into the generated plugin at runtime, so the same build can serve multiple deployments with different third-party IDs via NUXT_PUBLIC_SCRIPTS_GLOBALS_<KEY>_<FIELD> env vars. Typo detection from #778 is extended to the new GLOBALS_ env-var path: configured globals keys are passed to validateScriptsEnvVars and unrecognised vars get suggestions (e.g. TRUSTED_SHOP_SRC -> trustedShops). Resolves #759.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR implements runtime-configurable globals for the Nuxt scripts module, enabling single-build multi-deployment scenarios. It extends environment variable validation to recognize Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/script/src/validate-env.ts (1)
61-62:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Early return prevents globals validation when no registry scripts exist.
The early return at line 61-62 checks only
validByKey.size(registry scripts withenvDefaults), but the globals env-var validation block (lines 76-107) runs later in the same loop. If a project configures onlyscripts.globalswithout anyscripts.registryentries that haveenvDefaults,validByKeywill be empty and the function returns before validatingNUXT_PUBLIC_SCRIPTS_GLOBALS_*env vars.🔧 Proposed fix
- if (!validByKey.size) + if (!validByKey.size && !validGlobalsByScreaming.size) returnAdd a test case covering globals-only configuration (no registry scripts):
it('validates globals env vars when no registry scripts are configured', () => { process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_TYPO_SRC = 'x' removeKeys.push('NUXT_PUBLIC_SCRIPTS_GLOBALS_TYPO_SRC') const logger = makeLogger() validateScriptsEnvVars([], new Set(), logger, ['trustedShops']) expect(logger.warn).toHaveBeenCalledTimes(1) const msg = (logger.warn as any).mock.calls[0][0] as string expect(msg).toContain('NUXT_PUBLIC_SCRIPTS_GLOBALS_TYPO_SRC') })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/script/src/validate-env.ts` around lines 61 - 62, The early return guarded by checking validByKey.size in validateScriptsEnvVars prevents the globals env-var validation from running when there are no registry scripts with envDefaults, causing NUXT_PUBLIC_SCRIPTS_GLOBALS_* checks to be skipped for globals-only configurations; remove or change that early return so the globals block always executes (i.e., only skip registry-specific validation when validByKey is empty but still run the globals validation for scripts.globals), update logic around validByKey and the subsequent globals validation loop in validateScriptsEnvVars, and add the suggested test that sets process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_* to ensure globals-only cases are validated.
🧹 Nitpick comments (1)
test/e2e/issue-759-globals-env-override.test.ts (1)
10-10: 💤 Low valueConsider test isolation.
The env var is set at module-level (top-level), which means it persists for the entire test file. If other test cases are added to this file, they will inherit this env var. Consider using a
beforeAllhook or documenting that this fixture expects the env var to be set.However, for a single-test file focused on issue
#759, this is acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/issue-759-globals-env-override.test.ts` at line 10, The assignment to process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRC at module scope makes the env var persist across the whole test file; move this setup into a beforeAll hook (and restore/delete it in an afterAll or beforeEach/afterEach) so the single test remains isolated. Locate the top-level env set (process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRC) and change it to be configured inside beforeAll/beforeEach and cleaned up afterwards to avoid leaking state to other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/fixtures/issue-759/nuxt.config.ts`:
- Around line 3-4: The fixture comment uses the wrong environment variable
tokenization (NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTEDSHOPS_SRC); update the comment
to the correct env-var name NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRC so it
matches the project's TRS_TOKENIZATION style and example usage in the feature
path (update the string in the comment in nuxt.config.ts).
---
Outside diff comments:
In `@packages/script/src/validate-env.ts`:
- Around line 61-62: The early return guarded by checking validByKey.size in
validateScriptsEnvVars prevents the globals env-var validation from running when
there are no registry scripts with envDefaults, causing
NUXT_PUBLIC_SCRIPTS_GLOBALS_* checks to be skipped for globals-only
configurations; remove or change that early return so the globals block always
executes (i.e., only skip registry-specific validation when validByKey is empty
but still run the globals validation for scripts.globals), update logic around
validByKey and the subsequent globals validation loop in validateScriptsEnvVars,
and add the suggested test that sets process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_*
to ensure globals-only cases are validated.
---
Nitpick comments:
In `@test/e2e/issue-759-globals-env-override.test.ts`:
- Line 10: The assignment to
process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRC at module scope makes
the env var persist across the whole test file; move this setup into a beforeAll
hook (and restore/delete it in an afterAll or beforeEach/afterEach) so the
single test remains isolated. Locate the top-level env set
(process.env.NUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRC) and change it to be
configured inside beforeAll/beforeEach and cleaned up afterwards to avoid
leaking state to other tests.
🪄 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: 2307891b-3051-49fc-9a78-33eddbf8a586
📒 Files selected for processing (10)
docs/content/docs/1.guides/4.global.mdpackages/script/src/module.tspackages/script/src/templates.tspackages/script/src/validate-env.tstest/e2e/issue-759-globals-env-override.test.tstest/fixtures/issue-759/app.vuetest/fixtures/issue-759/nuxt.config.tstest/fixtures/issue-759/package.jsontest/unit/templates.test.tstest/unit/validate-env.test.ts
Address CodeRabbit feedback: the early return guarded only by validByKey.size skipped GLOBALS_ validation for projects that configure only scripts.globals without any registry script that declares envDefaults. Also tighten the fixture comment to use the correct env-var casing. Adds regression test for the globals-only path.
🔗 Linked issue
Resolves #759
❓ Type of change
📚 Description
Today,
scripts.globalsentries are baked into the bundle at build time, so the same build can't serve multiple deployments with different third-party IDs (Trusted Shops, Awin, GTM, etc.) without rebuilding.This change exposes each global's input via
runtimeConfig.public.scriptsGlobalsand has the generated plugin merge the runtime config over the build-time default. The result: any field (src,integrity, customdata-*attrs) is overridable per deployment via standard Nuxt env vars:The typo-detection added in #778 is extended to the new
GLOBALS_env-var path: configured globals keys flow intovalidateScriptsEnvVars, and unrecognised vars get suggestions (e.g.TRUSTED_SHOP_SRC→trustedShops).scriptOptions(triggers, mode) and object-form triggers stay build-time only — they can't round-trip through env vars. Asset bundling is silently skipped for env-overridable globals (the bundle transformer's static AST analysis can't see through the runtime-config wrapper); documented indocs/content/docs/1.guides/4.global.md.Verified end-to-end: a new e2e fixture (
test/fixtures/issue-759/) boots Nuxt withNUXT_PUBLIC_SCRIPTS_GLOBALS_TRUSTED_SHOPS_SRCset and asserts the rendered<link href>reflects the env value, not the build default.