fix(first-party): inject proxy endpoints for boolean/mock registry entries#640
fix(first-party): inject proxy endpoints for boolean/mock registry entries#640
Conversation
|
@oritwoen is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
📝 WalkthroughWalkthroughThe PR changes auto-injection logic in src/first-party/auto-inject.ts: registry entries are now skipped only when falsy; object entries (or first element of arrays) are processed as configs, while Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/first-party/auto-inject.ts`:
- Around line 57-68: The bug is that when handling shorthand entries (entry ===
true || entry === 'mock') the code sets config = {} so
def.computeValue(collectPrefix, config) cannot see
runtimeConfig.public.scripts.posthog.region; change the shorthand branch to set
config to the runtime config for that integration (e.g. config =
runtimeConfig.public?.scripts?.posthog ?? {}) or merge the runtime region into
the empty config before calling def.computeValue so def.computeValue and
def.configField see the runtime-provided region; then add a regression test that
uses a shorthand entry (posthog: true) with rt.public.scripts.posthog.region =
'eu' and assert the injected path uses /ph-eu.
🪄 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: 12cc6340-5149-495a-aa4d-f796acac5faa
📒 Files selected for processing (2)
src/first-party/auto-inject.tstest/unit/auto-inject.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/first-party/auto-inject.ts (1)
76-84: Consider adding a fallback when runtimeConfig entry is missing for boolean/mock entries.For boolean/
'mock'entries, ifruntimeConfig.public.scripts[registryKey]doesn't exist (e.g.,rtEntryis undefined), the computed value is never assigned anywhere—lines 77 and 83 are both skipped.In practice, this is mitigated by
module.ts(lines 347-350) which populates runtimeConfig fromREGISTRY_ENV_DEFAULTSbefore this function is called. However, as a defensive measure, consider initializing the runtimeConfig entry if it's missing.🛡️ Optional defensive initialization
if (typeof entry === 'object') { config[def.configField] = value } // Propagate to runtimeConfig - if (rtEntry && typeof rtEntry === 'object') { - if (rtConfig) - rtConfig[def.configField] = value + if (rtScripts) { + if (rtEntry && typeof rtEntry === 'object') { + if (rtConfig) + rtConfig[def.configField] = value + } + else if (entry === true || entry === 'mock') { + // Initialize runtimeConfig entry if missing for boolean/mock entries + rtScripts[def.registryKey] = { [def.configField]: value } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/first-party/auto-inject.ts` around lines 76 - 84, The computed boolean/'mock' values are never written when rtEntry is missing; modify the logic in auto-inject.ts around config[def.configField], rtEntry, rtConfig and registryKey to defensively initialize the runtimeConfig entry: if rtEntry is falsy and rtConfig exists, create rtConfig.public ||= { scripts: {} } (or the minimal path needed) and set rtConfig.public.scripts[registryKey] (or rtConfig[def.configField] as appropriate) to the computed value before or instead of the existing conditional, so boolean/'mock' entries always propagate to runtimeConfig even when the rtEntry was undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/first-party/auto-inject.ts`:
- Around line 76-84: The computed boolean/'mock' values are never written when
rtEntry is missing; modify the logic in auto-inject.ts around
config[def.configField], rtEntry, rtConfig and registryKey to defensively
initialize the runtimeConfig entry: if rtEntry is falsy and rtConfig exists,
create rtConfig.public ||= { scripts: {} } (or the minimal path needed) and set
rtConfig.public.scripts[registryKey] (or rtConfig[def.configField] as
appropriate) to the computed value before or instead of the existing
conditional, so boolean/'mock' entries always propagate to runtimeConfig even
when the rtEntry was undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66935aa2-2ce0-4132-8336-3f15873c2642
📒 Files selected for processing (2)
src/first-party/auto-inject.tstest/unit/auto-inject.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/auto-inject.test.ts
1 participant
autoInjectProxyEndpointsskips registry scripts configured with boolean shorthand (posthog: true) or mock mode (posthog: 'mock'). Thetypeof entry !== 'object'guard bails before injectingapiHost/endpoint/hostUrlinto runtimeConfig, so the SDK connects directly to third-party servers instead of going through the first-party proxy.Traced the flow:
module.tscreatesruntimeConfig.public.scripts.posthog = { apiKey: '' }fromREGISTRY_ENV_DEFAULTS, butconfig.registry.posthogstaystrue. WhenautoInjectProxyEndpointsruns, it seestypeof true !== 'object'and skips. The runtimeConfig entry never getsapiHost, so PostHog SDK defaults tohttps://us.i.posthog.com.The fix handles
trueand'mock'entries by computing the proxy endpoint value against a temporary empty config, then writing only to the runtimeConfig entry. The boolean in the registry stays untouched.Affects all five scripts with auto-inject definitions: PostHog, Plausible, Umami, Rybbit, Databuddy.
Added 16 unit tests covering object/array/boolean/mock entries, empty arrays, falsy entries, custom prefixes, and EU region handling.