fix(analytics): replace GTM container and remove duplicate GA4 pageview tracking#3041
Open
fix(analytics): replace GTM container and remove duplicate GA4 pageview tracking#3041
Conversation
…ew tracking
The old GTM container (GTM-WT4KSXNQ) belongs to Antidote and is not configurable
by our team. An inline gtag snippet was also sending pageviews to the same GA4
property (G-LFRGN2J2RV), inflating page views and user counts.
- Replace GTM ID with new container GTM-MSF384N3
- Remove inline GAnalytics trackPageViews component
- Change gtag("config") to gtag("set") for user identification to avoid triggering extra pageviews
Contributor
📝 WalkthroughWalkthroughUpdated GTM ID, removed the CustomGoogleAnalytics component and its top-level usage, replaced Google Analytics client calls with direct writes to Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client App
participant Service as AnalyticsService
participant DL as window.dataLayer
participant GTM as Google Tag Manager
rect rgba(135,206,250,0.5)
App->>Service: identify(user)
Service->>DL: push { user_id: <id> }
DL->>GTM: GTM reads dataLayer
end
rect rgba(144,238,144,0.5)
App->>Service: track(event, props)
Service->>DL: push { event: <name>, ...props }
DL->>GTM: GTM reads event and handles tags
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Comment |
…o TrackingScripts Move web vitals reporting into TrackingScripts and delete the now-redundant CustomGoogleAnalytics component.
Web vitals must run at the _app level to cover all pages, not just those using Layout. TrackingScripts in Layout now only handles GTM and Growth Channel scripts.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3041 +/- ##
==========================================
+ Coverage 59.62% 59.64% +0.02%
==========================================
Files 1034 1033 -1
Lines 24248 24239 -9
Branches 6009 6005 -4
==========================================
Hits 14458 14458
+ Misses 8539 8531 -8
+ Partials 1251 1250 -1
🚀 New features to boost your workflow:
|
…r GA events GTM now handles GA4 directly, so the nextjs-google-analytics package is no longer needed. Custom events are pushed to window.dataLayer instead, which GTM picks up natively. Also removes useReportWebVitals — web vitals can be measured via GTM's built-in Core Web Vitals trigger.
…ibility
GTM does not expose window.gtag — it only reads from window.dataLayer.
Replace gtag("set", { user_id }) with dataLayer.push({ user_id }) so
user identification works correctly with the new GTM container.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
apps/deploy-web/src/services/analytics/analytics.service.spec.ts (1)
95-111:⚠️ Potential issue | 🟡 MinorAssert the exact
identify()payload here.
toContainEqualstill passes ifidentify()pushes{ user_id }and an extra GTM event, so this doesn’t fully cover the “no extra pageview on identify” regression. An exact array assertion would lock that down.Suggested assertion
- expect(dataLayer).toContainEqual({ user_id: user.id }); + expect(dataLayer).toEqual([{ user_id: user.id }]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.spec.ts` around lines 95 - 111, Replace the loose assertion on dataLayer with an exact array equality so we lock down the identify payload: assert that dataLayer equals exactly [{ user_id: user.id }] (e.g., replace the toContainEqual check with a toEqual-style assertion) after calling service.identify(user), and keep/also assert the amplitude mock identify/setUserId were not invoked when amplitude is disabled (identify, setUserId) to ensure no extra GTM/pageview events are pushed by service.identify.
🧹 Nitpick comments (1)
apps/deploy-web/src/services/analytics/analytics.service.ts (1)
156-156: InitializedataLayerbefore the first push.This getter can still return
undefined, so both newpush()paths silently no-op until GTM creates the array. I’d harden this here withwindow.dataLayer ??= []instead of relying onapps/deploy-web/src/components/layout/TrackingScripts.tsxboot order.Suggested hardening
- private readonly getDataLayer: () => Record<string, unknown>[] | undefined = () => (isBrowser ? window.dataLayer : undefined), + private readonly getDataLayer: () => Record<string, unknown>[] | undefined = () => { + if (!isBrowser) { + return undefined; + } + + window.dataLayer ??= []; + return window.dataLayer; + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.ts` at line 156, The getDataLayer getter can return undefined causing push() to no-op; modify getDataLayer (and any code paths that call it) to ensure window.dataLayer is initialized before use by assigning window.dataLayer ??= [] when isBrowser is true so the getter always returns a Record<string, unknown>[]; update the getter named getDataLayer to perform this initialization and then return window.dataLayer to guarantee pushes succeed even if GTM script hasn't run yet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/deploy-web/src/services/analytics/analytics.service.ts`:
- Around line 255-256: The current push uses { event: name, ...props } which
allows a caller-supplied event field in eventProperties to override the
transformed name; update the call in the analytics service (reference
transformGaEvent and getDataLayer) to ensure the GTM event name cannot be
overridden by either (a) spread props first and set event: name last, or (b)
strip the event key from props before spreading (delete or omit the event
property) and then push { event: name, ...cleanProps } so GTM always receives
the intended trigger name.
---
Outside diff comments:
In `@apps/deploy-web/src/services/analytics/analytics.service.spec.ts`:
- Around line 95-111: Replace the loose assertion on dataLayer with an exact
array equality so we lock down the identify payload: assert that dataLayer
equals exactly [{ user_id: user.id }] (e.g., replace the toContainEqual check
with a toEqual-style assertion) after calling service.identify(user), and
keep/also assert the amplitude mock identify/setUserId were not invoked when
amplitude is disabled (identify, setUserId) to ensure no extra GTM/pageview
events are pushed by service.identify.
---
Nitpick comments:
In `@apps/deploy-web/src/services/analytics/analytics.service.ts`:
- Line 156: The getDataLayer getter can return undefined causing push() to
no-op; modify getDataLayer (and any code paths that call it) to ensure
window.dataLayer is initialized before use by assigning window.dataLayer ??= []
when isBrowser is true so the getter always returns a Record<string, unknown>[];
update the getter named getDataLayer to perform this initialization and then
return window.dataLayer to guarantee pushes succeed even if GTM script hasn't
run yet.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29ec08dd-6db6-477f-9747-937631eac8ef
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
apps/deploy-web/package.jsonapps/deploy-web/src/pages/_app.tsxapps/deploy-web/src/services/analytics/analytics.service.spec.tsapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/types/global.ts
💤 Files with no reviewable changes (2)
- apps/deploy-web/src/pages/_app.tsx
- apps/deploy-web/package.json
✅ Files skipped from review due to trivial changes (1)
- apps/deploy-web/src/types/global.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/deploy-web/src/services/analytics/analytics.service.ts (1)
156-156: Initializewindow.dataLayeras fallback in the default getter.Line 156 returns
undefinedifwindow.dataLayeris not yet defined, causing Lines 180 and 256 to silently skip events via optional chaining. AlthoughisBrowserguards prevent server-side calls, initializing the queue here ensures earlyidentify()andtrack()invocations buffer to the dataLayer even if GTM hasn't loaded, matching GTM's native pre-bootstrap behavior.Suggested fix
- private readonly getDataLayer: () => Record<string, unknown>[] | undefined = () => (isBrowser ? window.dataLayer : undefined), + private readonly getDataLayer: () => Record<string, unknown>[] | undefined = () => { + if (!isBrowser) { + return undefined; + } + + window.dataLayer = window.dataLayer || []; + return window.dataLayer; + },Also applies to: 179-180, 255-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/services/analytics/analytics.service.ts` at line 156, The getter getDataLayer should initialize a client-side fallback queue instead of returning undefined so early identify() and track() calls buffer until GTM loads; modify the getDataLayer implementation to, when isBrowser is true, ensure window.dataLayer exists (e.g., create an empty array if undefined) and return it (update the return type if necessary), and rely on that initialized array in the identify() and track() call sites so optional chaining no longer silently drops events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/deploy-web/src/services/analytics/analytics.service.ts`:
- Line 156: The getter getDataLayer should initialize a client-side fallback
queue instead of returning undefined so early identify() and track() calls
buffer until GTM loads; modify the getDataLayer implementation to, when
isBrowser is true, ensure window.dataLayer exists (e.g., create an empty array
if undefined) and return it (update the return type if necessary), and rely on
that initialized array in the identify() and track() call sites so optional
chaining no longer silently drops events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c78218b6-d6c3-4ede-b381-6a6b2d42f3ce
📒 Files selected for processing (1)
apps/deploy-web/src/services/analytics/analytics.service.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
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.
Why
The old GTM container (
GTM-WT4KSXNQ) belongs to Antidote and we have no access to configure it. Additionally, an inline<GAnalytics trackPageViews />component was sending pageviews directly to GA4 (G-LFRGN2J2RV) alongside GTM — double-counting page views, user counts, and funnel metrics.A new GTM container (
GTM-MSF384N3) has been created that we own and can configure. It handles GA4 directly, so the inline gtag snippet is no longer needed.What
GTM-WT4KSXNQ→GTM-MSF384N3inapps/deploy-web/env/.env.production<GAnalytics trackPageViews />fromCustomGoogleAnalytics.tsx— the new GTM container handles GA4 pageviews directlygtag("config", measurementId, { user_id })togtag("set", { user_id })inAnalyticsService.identify()to avoid triggering an extra pageview on every identify callNo changes to: dataLayer initialization, custom event tracking, Amplitude, web vitals reporting, or any other tracking.
Summary by CodeRabbit