Fides.gcm - Define gtag as a fallback when its not already defined#7543
Open
Fides.gcm - Define gtag as a fallback when its not already defined#7543
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
jjdaurora
requested review from
thabofletcher and
tvandort
and removed request for
speaker-ender
jjdaurora
changed the title
Contributor
Greptile SummaryThis PR modifies
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: 8a08744 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tvandort
requested changes
Mar 3, 2026
|
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:
📝 WalkthroughWalkthroughAdds an ambient Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
clients/fides-js/src/integrations/gcm.ts (1)
158-178:setGoogleConsentDefaultsis currently unused.This helper is defined but never invoked in this file. Please either wire it into the initialization flow or remove it to avoid dead code drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/fides-js/src/integrations/gcm.ts` around lines 158 - 178, The helper setGoogleConsentDefaults is defined but never used; either wire it into the GCM initialization flow (call setGoogleConsentDefaults(consent, mapping) from the module's initialization function such as initializeGcm / configureGcm where consent/mapping are available so default consent is pushed to gtag/dataLayer), or remove the function and its export entirely to avoid dead code—ensure you also update any related tests and exports if you choose removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/fides-js/src/integrations/gcm.ts`:
- Around line 163-169: The guard verifies window.gtag is not a function but then
uses nullish coalescing which can leave a non-function value in place; change
both gtag fallback blocks so they unconditionally assign a proper fallback: set
window.dataLayer to a fallback array via assignment (e.g. window.dataLayer =
window.dataLayer || []) and assign window.gtag to the fallback function directly
(e.g. window.gtag = function gtag(){ dataLayer.push(arguments) }) instead of
using ??, and apply the same change in the second fallback block that references
window.gtag/window.dataLayer.
- Line 165: Add the missing dataLayer property to the global Window augmentation
so TypeScript knows about window.dataLayer; update the existing declare global {
interface Window { gtag?: GtagFunction; dataLayer?: any[]; } } declaration in
this file (the same augmentation that currently declares gtag) so references to
window.dataLayer in the gcm integration (accesses at window.dataLayer =
window.dataLayer ?? [] and the later usage) no longer produce type errors.
---
Nitpick comments:
In `@clients/fides-js/src/integrations/gcm.ts`:
- Around line 158-178: The helper setGoogleConsentDefaults is defined but never
used; either wire it into the GCM initialization flow (call
setGoogleConsentDefaults(consent, mapping) from the module's initialization
function such as initializeGcm / configureGcm where consent/mapping are
available so default consent is pushed to gtag/dataLayer), or remove the
function and its export entirely to avoid dead code—ensure you also update any
related tests and exports if you choose removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6cd571e-5281-4f62-b9fc-a4cf40bf64d4
📒 Files selected for processing (1)
clients/fides-js/src/integrations/gcm.ts
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)
clients/fides-js/src/integrations/gcm.ts (1)
154-175:⚠️ Potential issue | 🟠 MajorFix CI-blocking lint errors in the fallback block.
Line 171 currently fails ESLint (
prefer-destructuring), and this block also triggers the reported formatting/lint warnings. Please adjust this section so frontend checks pass.Suggested patch
-/** - * - * Push consent to Google Consent Mode v2 - */ +/** + * Push consent to Google Consent Mode v2 + */ const pushConsentToGtag = ( consent: NoticeConsent, options: GcmOptions, ): void => { @@ if (typeof window.gtag !== "function") { // define dataLayer if it doesn't exist yet so that we can push to it in the gtag function fallback window.dataLayer ??= []; - const dataLayer = window.dataLayer; + const { dataLayer } = window; // define gtag if it doesn't exist and push to dataLayer - window.gtag = function gtag(){ - dataLayer.push(arguments) - } + window.gtag = function gtag(...args: unknown[]) { + dataLayer!.push(args); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/fides-js/src/integrations/gcm.ts` around lines 154 - 175, The fallback block in pushConsentToGtag triggers ESLint prefer-destructuring and formatting errors; change window.dataLayer ??= []; followed by const dataLayer = window.dataLayer; to use destructuring: window.dataLayer ??= []; const { dataLayer } = window; and reformat the gtag fallback to a properly spaced and terminated function (e.g., function gtag() { dataLayer.push(arguments); }) so linting/formatting rules pass while preserving behavior of pushConsentToGtag and the window.gtag fallback.
🤖 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 `@clients/fides-js/src/integrations/gcm.ts`:
- Around line 154-175: The fallback block in pushConsentToGtag triggers ESLint
prefer-destructuring and formatting errors; change window.dataLayer ??= [];
followed by const dataLayer = window.dataLayer; to use destructuring:
window.dataLayer ??= []; const { dataLayer } = window; and reformat the gtag
fallback to a properly spaced and terminated function (e.g., function gtag() {
dataLayer.push(arguments); }) so linting/formatting rules pass while preserving
behavior of pushConsentToGtag and the window.gtag fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4d8a7bc-f2be-4eab-ad29-dd4cac3231f2
📒 Files selected for processing (1)
clients/fides-js/src/integrations/gcm.ts
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
2 participants
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.
Ticket ENG-2849
Description Of Changes
Fides.gcmmethod that integrates with Google Consent Modegtagwhen it is not defined, instead of logging a warning and returning early.gtagload prior to the method call.Code Changes
Steps to Confirm
!-- list any manual steps for reviewers to confirm the changes -->
fides, loadfides-js-demo.htmlwheregtagcurrently doesn't loadFides.gcmto the pagedataLayerqueuePre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and worksSummary by CodeRabbit