feat(gtm,ga): consent.default() + strict GCMv2 validation#772
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
harlan-zw
changed the title
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:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR unifies consent handling for Google Analytics and Google Tag Manager by introducing a shared GcmConsentApi type contract with strict schema validation. A new _gcm-consent.ts module exports validation and attachment helpers that wire consent.default() and consent.update() onto script instances. The Google Analytics and Google Tag Manager registry implementations now use the shared API and forward consent actions via gcmConsent.push handlers. The registry utilities enable opt-in GCM consent integration, and documentation and tests are updated to cover runtime-derived defaults and validation warnings. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 (2)
docs/content/docs/1.guides/3.consent.md (1)
92-92: 💤 Low valueConsider aligning validation wording with vendor-specific docs.
Line 92 describes validation as "validated and warned via
consola", while the GA and GTM docs specify "validated against the canonical GCMv2 schema and warned viaconsola". Adding "against the canonical GCMv2 schema" here would improve consistency across the documentation set.📝 Suggested wording
-Every consent-aware `useScriptX()`{lang="ts"} returns a `consent` object typed to the vendor's native API. Combine it with `defaultConsent` for the initial policy (applied in `clientInit` before the vendor fires its first call) and call `consent.*` from your cookie banner to update. For GCMv2 scripts (Google Analytics, Google Tag Manager), `consent.default(state)`{lang="ts"} is also available for runtime-derived defaults; unknown GCMv2 keys and non-`granted`/`denied` values are validated and warned via `consola`. +Every consent-aware `useScriptX()`{lang="ts"} returns a `consent` object typed to the vendor's native API. Combine it with `defaultConsent` for the initial policy (applied in `clientInit` before the vendor fires its first call) and call `consent.*` from your cookie banner to update. For GCMv2 scripts (Google Analytics, Google Tag Manager), `consent.default(state)`{lang="ts"} is also available for runtime-derived defaults; unknown GCMv2 keys and non-`granted`/`denied` values are validated against the canonical GCMv2 schema and warned via `consola`.🤖 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 `@docs/content/docs/1.guides/3.consent.md` at line 92, Update the phrasing in the consent docs to match vendor docs: change the sentence about validation to state that unknown GCMv2 keys and non-`granted`/`denied` values are "validated against the canonical GCMv2 schema and warned via `consola`", preserving references to useScriptX(), `defaultConsent`, and `consent.default(state)` so readers can locate the related examples and behavior.packages/script/src/runtime/registry/google-analytics.ts (1)
135-137: ⚡ Quick winUse shared
ConsentStatein the transport callback to avoid type drift.
gcmConsent.pushcurrently typesstateas localConsentOptions. Using the sharedConsentStatehere keeps the transport contract aligned with the canonical consent schema.Suggested patch
-import type { RegistryScriptInput, UseScriptContext } from '#nuxt-scripts/types' +import type { ConsentState, RegistryScriptInput, UseScriptContext } from '#nuxt-scripts/types' @@ gcmConsent: { - push: (proxy: any, action: 'default' | 'update', state: ConsentOptions) => (proxy as GoogleAnalyticsApi).gtag('consent', action, state), + push: (proxy: any, action: 'default' | 'update', state: ConsentState) => (proxy as GoogleAnalyticsApi).gtag('consent', action, state), },🤖 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/runtime/registry/google-analytics.ts` around lines 135 - 137, The transport callback gcmConsent.push currently types its state parameter as the local ConsentOptions, causing type drift; update the signature to use the shared ConsentState type instead (replace ConsentOptions with ConsentState in the gcmConsent.push declaration and any related imports/usage), ensuring the transport contract matches the canonical consent schema and updating any import statements to reference the module that exports ConsentState.
🤖 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.
Nitpick comments:
In `@docs/content/docs/1.guides/3.consent.md`:
- Line 92: Update the phrasing in the consent docs to match vendor docs: change
the sentence about validation to state that unknown GCMv2 keys and
non-`granted`/`denied` values are "validated against the canonical GCMv2 schema
and warned via `consola`", preserving references to useScriptX(),
`defaultConsent`, and `consent.default(state)` so readers can locate the related
examples and behavior.
In `@packages/script/src/runtime/registry/google-analytics.ts`:
- Around line 135-137: The transport callback gcmConsent.push currently types
its state parameter as the local ConsentOptions, causing type drift; update the
signature to use the shared ConsentState type instead (replace ConsentOptions
with ConsentState in the gcmConsent.push declaration and any related
imports/usage), ensuring the transport contract matches the canonical consent
schema and updating any import statements to reference the module that exports
ConsentState.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16f935e4-d9ad-4a08-ad60-31bf9f0372b9
📒 Files selected for processing (10)
docs/content/docs/1.guides/3.consent.mddocs/content/scripts/google-analytics.mddocs/content/scripts/google-tag-manager.mdpackages/script/src/runtime/registry/_gcm-consent.tspackages/script/src/runtime/registry/google-analytics.tspackages/script/src/runtime/registry/google-tag-manager.tspackages/script/src/runtime/registry/schemas.tspackages/script/src/runtime/types.tspackages/script/src/runtime/utils.tstest/nuxt-runtime/consent-default.nuxt.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/nuxt-runtime/consent-default.nuxt.test.ts (1)
362-385: ⚡ Quick winExpand strict-validation tests to cover invalid values and blocked forwarding.
These tests currently validate unknown-key warnings only. Add cases for invalid values (not
granted/denied) and assert invalid payloads are not forwarded todataLayer, so strict GCMv2 behavior is protected end-to-end.Suggested test extension
+ it('gtm: validateConsentState warns and blocks invalid GCMv2 values in consent.update()', async () => { + ;(window as any).dataLayer = [] + const { logger } = await import('../../packages/script/src/runtime/logger') as any + logger.warn.mockClear() + const { useScriptGoogleTagManager } = await import('../../packages/script/src/runtime/registry/google-tag-manager') + const result: any = useScriptGoogleTagManager({ id: 'GTM-XXXX' }) + result._opts.clientInit() + + const before = ((window as any).dataLayer as any[]).length + result.consent.update({ ad_storage: 'invalid' } as any) + + expect(logger.warn).toHaveBeenCalled() + expect(((window as any).dataLayer as any[]).length).toBe(before) + })🤖 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/nuxt-runtime/consent-default.nuxt.test.ts` around lines 362 - 385, Add tests that besides checking unknown-key warnings also submit GCMv2 payloads containing invalid values (e.g., not "granted"/"denied") to both useScriptGoogleTagManager(...).consent.default(...) and .consent.update(...), and assert logger.warn is called with the offending key/value (use the existing logger.warn mock and expect.stringContaining). Also assert that invalid payloads are not forwarded to window.dataLayer (verify dataLayer stays empty or unchanged after calling result._opts.clientInit() and then result.consent.default(...) / result.consent.update(...)). Reuse the same setup steps (window.dataLayer = [], import logger, call useScriptGoogleTagManager and result._opts.clientInit) and add these new expectations alongside the existing unknown-key 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.
Nitpick comments:
In `@test/nuxt-runtime/consent-default.nuxt.test.ts`:
- Around line 362-385: Add tests that besides checking unknown-key warnings also
submit GCMv2 payloads containing invalid values (e.g., not "granted"/"denied")
to both useScriptGoogleTagManager(...).consent.default(...) and
.consent.update(...), and assert logger.warn is called with the offending
key/value (use the existing logger.warn mock and expect.stringContaining). Also
assert that invalid payloads are not forwarded to window.dataLayer (verify
dataLayer stays empty or unchanged after calling result._opts.clientInit() and
then result.consent.default(...) / result.consent.update(...)). Reuse the same
setup steps (window.dataLayer = [], import logger, call
useScriptGoogleTagManager and result._opts.clientInit) and add these new
expectations alongside the existing unknown-key tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4127be36-2cf1-42d4-b94b-f9c153b527bd
📒 Files selected for processing (4)
docs/content/docs/1.guides/3.consent.mddocs/content/scripts/google-analytics.mddocs/content/scripts/google-tag-manager.mdtest/nuxt-runtime/consent-default.nuxt.test.ts
✅ Files skipped from review due to trivial changes (3)
- docs/content/docs/1.guides/3.consent.md
- docs/content/scripts/google-analytics.md
- docs/content/scripts/google-tag-manager.md
…t validation
- Add `GcmConsentApi` ({ default, update }) and a shared `gcmConsent`
opt-in contract for registry scripts. `useRegistryScript` auto-attaches
`consent.default/update` post-creation and runs every call through the
canonical GCMv2 schema, warning on unknown keys / bad values via consola.
- `gcmConsentState` upgraded to `strictObject` so typo'd keys
(e.g. `analytic_storage`) and non-`granted`/`denied` values are caught.
- GTM + GA refactored to declare `gcmConsent: { push }` instead of
hand-rolling their own consent attachment. Removes the double
`onBeforeGtmStart` invocation on first init.
- Docs updated to document `consent.default()` and the strict validation.
Leaked in from an unrelated debug-mode branch; the consent test only exercises the GCMv2 validation path, which uses logger.warn (always emitted), so the build-time debug constant is unnecessary here.
…tion - GTM consent.default() queues an Arguments object on dataLayer (the gtag.js contract from #771), mirroring the existing consent.update() Arguments assertion. - GA consent.default() pushes gtag('consent','default', state). - validateConsentState warns on bad GCMv2 *values* (e.g. `ad_storage: 'allow'`), not just unknown keys.
harlan-zw
force-pushed
the
feat/gcm-consent-contract
branch
from
ef4f65c to
3b2754b
Compare
- GA gcmConsent.push now types `state` as the canonical `ConsentState` (cast to local `ConsentOptions` at the gtag boundary) to keep the contract aligned across vendors. - Consent guide L92 mirrors the vendor-doc phrasing: "validated against the canonical GCMv2 schema and warned via `consola`".
Collaborator
Author
|
Addressed two of three CodeRabbit nits in
Skipping "invalid payloads should be blocked from forwarding": validation is intentionally non-blocking — silently dropping a malformed payload would be worse UX than warning + forwarding (the existing dataLayer behaviour is what users debug against). The new |
`gcmConsentState` reverts to `object()` so `defaultConsent` schema parses don't throw on typos (avoiding a dev-only soft break for misconfigured users). The runtime `consent.*` API rebuilds a `strictObject` from the same entries, so unknown keys / non-`granted`/`denied` values still surface as a `consola` warning — just without crashing the dev parse.
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 #745 (discussion) — the consent debugging confusion that motivated tightening this surface.
❓ Type of change
📚 Description
GA + GTM each hand-rolled their own
consent.update()attachment, and unknown GCMv2 keys / non-granted/deniedvalues failed silently. This adds a sharedgcmConsentopt-in contract onuseRegistryScript— both scripts now declaregcmConsent: { push }and get a typedconsent.default/updateAPI attached automatically, with every call validated against the canonical schema (gcmConsentStateupgraded tostrictObject) and warned via consola on bad input. As a bonus this fixes the GTMonBeforeGtmStartdouble-invocation on first init.