fix(registry): add missing Google Ads proxy domains (www.google.com, www.googletagmanager.com)#678
fix(registry): add missing Google Ads proxy domains (www.google.com, www.googletagmanager.com)#678felixgabler wants to merge 3 commits intonuxt:mainfrom
Conversation
…gle Analytics proxy domains Google Ads gtag makes runtime requests to www.google.com for conversion tracking (/pagead/1p-conversion/, /pagead/1p-user-list/, /g/collect) and www.googletagmanager.com for tag discovery (/td). When the first-party proxy is enabled, the AST rewriter intercepts these URLs but the server handler rejects them with 403 because the domains are not in the allowlist. Fixes nuxt#677
|
@felixgabler is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedPull request was closed or merged during review 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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis change reformats Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 warnings)
✅ 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 |
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/script/src/registry.ts (1)
1-1101:⚠️ Potential issue | 🟠 MajorFix widespread formatting violations introduced by this change.
The entire file has been reformatted with semicolons and 4-space indentation, which violates the project's ESLint configuration. Static analysis reports over 1000 style violations including:
- Extra semicolons (project expects none)
- 4-space indentation (project expects 2 spaces)
- Incorrect member delimiters in type definitions
- Various operator placement and bracket style issues
This large formatting change obscures the small functional fix (adding two domains) and will likely cause CI/CD failures.
🔧 Recommended fix
Revert the formatting changes and keep only the functional modification:
- Discard all formatting changes
- Add only the two new domains to the
googleAnalyticsproxy configuration:
'www.google.com''www.googletagmanager.com'- Run the project's configured formatter/linter (
npm run lint:fixor equivalent) to ensure complianceAlternatively, if you have uncommitted changes, use:
git checkout HEAD -- packages/script/src/registry.ts # Then manually add only lines 963-964 with proper formatting npm run lint:fixThis will preserve the functional fix while maintaining code style consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/registry.ts` around lines 1 - 1101, Revert the wholesale formatting changes and keep only the functional change: in the registry() definition for 'googleAnalytics' (the def('googleAnalytics') entry in this file) add the two domains 'www.google.com' and 'www.googletagmanager.com' to its proxy.domains array, restore original project formatting (2-space indentation, no semicolons, original member delimiters) and then run the project's formatter/linter (e.g. npm run lint:fix) before committing; if you have local formatting changes to discard, reset the file to HEAD and reapply only the two domain additions to the googleAnalytics proxy block.
🤖 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/script/src/registry.ts`:
- Around line 1-1101: Revert the wholesale formatting changes and keep only the
functional change: in the registry() definition for 'googleAnalytics' (the
def('googleAnalytics') entry in this file) add the two domains 'www.google.com'
and 'www.googletagmanager.com' to its proxy.domains array, restore original
project formatting (2-space indentation, no semicolons, original member
delimiters) and then run the project's formatter/linter (e.g. npm run lint:fix)
before committing; if you have local formatting changes to discard, reset the
file to HEAD and reapply only the two domain additions to the googleAnalytics
proxy block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9aa0a0ad-95b7-4dc3-8967-a98dd869b7d9
📒 Files selected for processing (1)
packages/script/src/registry.ts
|
Sounds good. There were some formatting issues, so I just merged the domain fix on main to expedite this. |
Adds
www.google.comandwww.googletagmanager.comto the Google Analytics registry's proxy domain list.When
firstPartyproxy is enabled and Google Ads is configured alongside GA4 viauseScriptGoogleAnalytics, the gtag SDK makes runtime requests towww.google.com/pagead/1p-conversion/,www.google.com/g/collect, andwww.googletagmanager.com/td. The AST rewriter intercepts these URLs but the server handler rejects them with 403 because the domains are not in the allowlist.Impact: 111K+ errors in ~3 hours of production traffic.
Fixes #677