Skip to content

feat(analytics): add Google Tag Manager for hosted environments#3993

Merged
waleedlatif1 merged 1 commit intostagingfrom
waleedlatif1/add-gtm-hosted
Apr 6, 2026
Merged

feat(analytics): add Google Tag Manager for hosted environments#3993
waleedlatif1 merged 1 commit intostagingfrom
waleedlatif1/add-gtm-hosted

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

@waleedlatif1 waleedlatif1 commented Apr 6, 2026

Summary

  • Add GTM script and noscript tags to root layout
  • Gated behind isHosted so self-hosted/OSS deployments are unaffected

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 6, 2026 •
edited
Loading

PR Summary

Medium Risk
Adds third-party Google Tag Manager/Analytics scripts to the global RootLayout, which can impact performance/telemetry behavior, but is gated behind isHosted so self-hosted installs remain unchanged.

Overview
Adds Google Tag Manager and Google Analytics (gtag.js) loading to apps/sim/app/layout.tsx, including the required <noscript> iframe fallback.

The new scripts are injected via Next.js Script with afterInteractive strategy and are enabled only when isHosted is true, leaving OSS/self-hosted environments unaffected.

Reviewed by Cursor Bugbot for commit b04e815. Configure here.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 6, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 6, 2026 7:22pm

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 6, 2026 •
edited
Loading

Greptile Summary

Adds Google Tag Manager and Google Analytics 4 (gtag.js) scripts to the root layout, both gated behind the isHosted flag so self-hosted/OSS deployments are unaffected. The two previously flagged concerns (duplicated container IDs and inline styles on the noscript iframe) have been resolved: IDs are now extracted as module-level constants (GTM_ID, GA_ID) and the iframe uses Tailwind classes (invisible hidden) instead of inline styles.

  • GTM noscript fallback is correctly placed at the top of <body> per Google's recommendation.
  • Both script blocks use strategy='afterInteractive' to avoid blocking the page render.
  • The one remaining question is whether GTM and the direct gtag.js init are intentionally co-present — if GTM is also configured to fire GA4 events for G-DR7YBE70VS, analytics will be double-counted.

Confidence Score: 5/5

Safe to merge — change is minimal, cleanly gated behind isHosted, and prior P1 concerns are resolved.

All remaining findings are P2 (potential analytics misconfiguration that cannot be confirmed from the codebase alone). No runtime errors, security issues, or data-loss risks introduced.

apps/sim/app/layout.tsx — verify GTM container configuration does not also fire a GA4 tag for G-DR7YBE70VS to avoid double-counting.

Important Files Changed

Filename Overview
apps/sim/app/layout.tsx Adds GTM + GA4 scripts gated behind isHosted; previous concerns (duplicated IDs, inline styles) resolved; minor open question around GTM/GA4 co-existence.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant GTM as GTM (GTM-T7PHSRX5)
    participant GA4 as GA4 (G-DR7YBE70VS)
    participant ODS as OneDollarStats

    Note over Browser: isHosted = true
    Browser->>GTM: Load gtm.js (afterInteractive)
    GTM-->>GA4: Fire GA4 tag (if configured in container)
    Browser->>GA4: Load gtag/js?id=G-DR7YBE70VS (afterInteractive)
    Browser->>GA4: gtag('config', 'G-DR7YBE70VS') — direct init
    Note over GA4: ⚠️ Potential double-counting if GTM also fires GA4
    Browser->>ODS: Load stonks.js (defer)
    Note over Browser: isHosted = false → GTM & GA4 skipped, ODS still loads
Loading

Reviews (2): Last reviewed commit: "feat(analytics): add Google Tag Manager ..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps bot reviewed Apr 6, 2026
View reviewed changes
@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/add-gtm-hosted branch from 3b73fc2 to bbf0769 Compare April 6, 2026 19:19
@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/add-gtm-hosted branch from bbf0769 to 8ef25bd Compare April 6, 2026 19:20
@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/add-gtm-hosted branch from 8ef25bd to b04e815 Compare April 6, 2026 19:22
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

waleedlatif1 commented Apr 6, 2026

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

waleedlatif1 commented Apr 6, 2026

@cursor review

cursor[bot]
cursor bot reviewed Apr 6, 2026
View reviewed changes
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b04e815. Configure here.

@waleedlatif1 waleedlatif1 merged commit c18f023 into staging Apr 6, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/add-gtm-hosted branch April 6, 2026 19:26
@waleedlatif1 waleedlatif1 mentioned this pull request Apr 7, 2026
Merged
emir-karabeg pushed a commit that referenced this pull request Apr 7, 2026
@waleedlatif1 @emir-karabeg
feat(analytics): add Google Tag Manager and Google Analytics for host…
2b59b7d 
…ed environments (#3993)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1