fix(analytics): page view events capturing unnecessary data#2134
Merged
fix(analytics): page view events capturing unnecessary data#2134
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdded an exported analytics.pageView(pageTitle, opts?) API and PageViewOptions type; exposed on the AnalyticsInterface and singleton. Google Analytics and Meta Pixel inits were changed to disable automatic PageView emission (GA: send_page_view: false; Meta: fbq.disablePushState = true and adjusted noscript query). Introduced usePageViewTracking and wired pageView calls into authenticated split-layout routes, compose block factories, BlockLoader (non-nested/non-preview), Login, EmailSignUp, and InteractiveOnboarding. Removed Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/lib/analytics/analytics.ts`:
- Around line 133-151: The single try/catch around all analytics dispatches can
drop subsequent providers if one throws; update the block so each provider call
is protected individually (wrap gtag(...) in its own try/catch, wrap fbq(...) in
its own try/catch, and wrap posthog.instance.capture(...) in its own try/catch)
and log errors with provider-specific context (e.g.,
console.error('[Analytics][gtag] ...', err)) so one provider failing doesn't
prevent the others from running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 45d55e09-963b-4e1b-acdb-507ddbe213a6
📒 Files selected for processing (4)
js/app/packages/app/component/split-layout/componentRegistry.tsxjs/app/packages/app/lib/analytics/analytics.tsjs/app/packages/app/lib/analytics/providers.tsjs/app/packages/core/internal/BlockLoader.tsx
…ytics-pageview-event
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
js/app/packages/app/lib/analytics/analytics.ts (1)
142-160:⚠️ Potential issue | 🟠 MajorIsolate provider failures so one exception doesn’t drop the rest.
A single
try/catcharound GA + Meta + PostHog means one throw prevents remaining providers from receiving the page view.🔧 Suggested fix
- try { - gtag('event', 'page_view', { - page_title: pageTitle, - page_location: pageLocation, - page_path: pagePath, - }); - - fbq('track', 'PageView', { - content_name: pageTitle, - }); - - posthog.instance.capture('$pageview', { - $current_url: pageLocation, - $pathname: pagePath, - $title: pageTitle, - }); - } catch (e) { - console.error('[Analytics] Failed to send page_view:', e); - } + try { + gtag('event', 'page_view', { + page_title: pageTitle, + page_location: pageLocation, + page_path: pagePath, + }); + } catch (e) { + console.error('[Analytics][ga] Failed to send page_view:', e); + } + + try { + fbq('track', 'PageView', { + content_name: pageTitle, + }); + } catch (e) { + console.error('[Analytics][meta-pixel] Failed to send page_view:', e); + } + + try { + posthog.capture('$pageview', { + $current_url: pageLocation, + $pathname: pagePath, + $title: pageTitle, + }); + } catch (e) { + console.error('[Analytics][posthog] Failed to send page_view:', e); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/lib/analytics/analytics.ts` around lines 142 - 160, The current single try/catch around gtag, fbq and posthog.instance.capture causes one provider error to abort the others; update analytics.ts so each provider call (gtag('event', ...), fbq('track', ...), and posthog.instance.capture(...)) is wrapped in its own try/catch that logs provider-specific failures (include the error and provider name) and continues executing the remaining providers to ensure one failure doesn't prevent others from receiving the page_view.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/lib/analytics/analytics.ts`:
- Line 137: The guard uses an undefined variable "options" causing runtime
errors; change the check to reference the correct options object in scope (for
example replace `if (options.disabled) return;` with `if
(this.options?.disabled) return;` or `if (opts?.disabled) return;` to match the
existing variable name used by the surrounding function/class in analytics.ts),
ensuring you use the same identifier (e.g., this.options or opts or
analyticsOptions) that is defined in the enclosing scope.
- Line 153: The pageview capture call uses a non-existent property: replace the
incorrect posthog.instance.capture call with posthog.capture so it uses the
PostHog instance created by new PostHog(); update the invocation where
posthog.instance.capture(...) appears (matching the existing correct usage of
posthog.capture elsewhere in this module) to call posthog.capture(...) with the
same payload and error handling.
---
Duplicate comments:
In `@js/app/packages/app/lib/analytics/analytics.ts`:
- Around line 142-160: The current single try/catch around gtag, fbq and
posthog.instance.capture causes one provider error to abort the others; update
analytics.ts so each provider call (gtag('event', ...), fbq('track', ...), and
posthog.instance.capture(...)) is wrapped in its own try/catch that logs
provider-specific failures (include the error and provider name) and continues
executing the remaining providers to ensure one failure doesn't prevent others
from receiving the page_view.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5e75619-5242-4761-b04f-be8c23801e5f
📒 Files selected for processing (1)
js/app/packages/app/lib/analytics/analytics.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
js/app/packages/app/lib/analytics/analytics.ts (1)
142-160:⚠️ Potential issue | 🟠 MajorSingle catch block can drop page-view delivery to remaining providers.
The single
try/catcharound all three provider dispatches means ifgtagthrows, neitherfbqnorposthogwill receive the event. Each provider should be wrapped independently to ensure partial failures don't block other providers.🔧 Suggested fix
- try { - gtag('event', 'page_view', { - page_title: pageTitle, - page_location: pageLocation, - page_path: pagePath, - }); - - fbq('track', 'PageView', { - content_name: pageTitle, - }); - - posthog.capture('$pageview', { - $current_url: pageLocation, - $pathname: pagePath, - $title: pageTitle, - }); - } catch (e) { - console.error('[Analytics] Failed to send page_view:', e); - } + try { + gtag('event', 'page_view', { + page_title: pageTitle, + page_location: pageLocation, + page_path: pagePath, + }); + } catch (e) { + console.error('[Analytics] Failed to send page_view to ga:', e); + } + + try { + fbq('track', 'PageView', { + content_name: pageTitle, + }); + } catch (e) { + console.error('[Analytics] Failed to send page_view to meta-pixel:', e); + } + + try { + posthog.capture('$pageview', { + $current_url: pageLocation, + $pathname: pagePath, + $title: pageTitle, + }); + } catch (e) { + console.error('[Analytics] Failed to send page_view to posthog:', e); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/lib/analytics/analytics.ts` around lines 142 - 160, The current single try/catch around the three vendor calls (gtag, fbq, posthog) can stop later providers from receiving the page_view if an earlier call throws; change the implementation so each provider call (gtag('event','page_view', ...), fbq('track','PageView', ...), posthog.capture('$pageview', ...)) is wrapped in its own try/catch, logging the provider name and error (e.g., console.error('[Analytics][gtag] ...', e)) so a failure in one does not prevent the others from running.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/auth/Login.tsx`:
- Around line 33-34: Move the analytics.pageView('login') call out of the
component body and into a mount effect so it only runs once; specifically, in
Login (component in Login.tsx) wrap the call inside an onMount callback (same
pattern used in EmailAuth.tsx / usePageViewTracking) so
analytics.pageView('login') executes inside onMount(() => { ... }) instead of at
top-level of the component.
In `@js/app/packages/app/component/EmailAuth.tsx`:
- Around line 89-91: The analytics.pageView('signup') call is currently at
component top-level and will run on every reconciliation; move it into an
onMount() callback so it runs exactly once on mount. Specifically, inside the
EmailAuth component wrap analytics.pageView('signup') in onMount(() =>
analytics.pageView('signup')), import onMount from 'solid-js' if missing, and
follow the same pattern used in componentRegistry.tsx; keep useAnalytics() where
it is and only relocate the pageView invocation.
---
Duplicate comments:
In `@js/app/packages/app/lib/analytics/analytics.ts`:
- Around line 142-160: The current single try/catch around the three vendor
calls (gtag, fbq, posthog) can stop later providers from receiving the page_view
if an earlier call throws; change the implementation so each provider call
(gtag('event','page_view', ...), fbq('track','PageView', ...),
posthog.capture('$pageview', ...)) is wrapped in its own try/catch, logging the
provider name and error (e.g., console.error('[Analytics][gtag] ...', e)) so a
failure in one does not prevent the others from running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f95fc7a4-aa61-45c8-ae05-13b9c55aceb2
📒 Files selected for processing (5)
js/app/packages/app/component/EmailAuth.tsxjs/app/packages/app/component/auth/Login.tsxjs/app/packages/app/component/interactive-onboarding/InteractiveOnboarding.tsxjs/app/packages/app/component/split-layout/componentRegistry.tsxjs/app/packages/app/lib/analytics/analytics.ts
This was referenced Mar 30, 2026
Merged
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.
This PR changes the way page view events are captured.
Currently, the page view event is auto captured by the analytics providers but this causes each event to capture the DOM document title which contains data like document, channel, email, etc. names/titles. Besides this data being largely irrelevant because there can be an infinite amount of dynamic pages due to user generated content, this data can also be sensitive and should not be captured.
In this PR, the automatic page view event tracking is replaced with manual page view tracking for component and blocks