fix(dashboards): Only trigger echarts dispatch sync for visible widgets

[narsaynorath]

Uses an intersection observer to register echarts instances to groups when the charts are visible in the viewport. This way, we don't dispatch cursor sync calls to widgets that are not visible, bypassing some of the load echarts needs to do to properly place the crosshairs

This only impacts hover performance when a user has scrolled and triggered all of the widgets to load in, which we expect users to scroll on this page. I crudely profiled this interaction by initializing all of the widgets on the page and then hovering over and around a single timeseries chart to show a difference in magnitude over multiple events. This was done with a production build locally (i.e. NODE_ENV=production pnpm dev-ui) and I consistently witnessed better performance with the new strategy.

These screenshots following are filtered to mousemove events

Before:

After:

[narsaynorath]

@gggritso could I get your review on this? I think you have the most context about widget sync. I think this should be a relatively simple change, in my testing it seems like it's helping with hovering perf because it minimizes how much we need to dispatch to other charts since they're not included in the groups

[sentry]

Bug: A race condition can cause the register function to be called before the IntersectionObserver is initialized, preventing charts from being observed and correctly removed from the sync group.
_{Severity: MEDIUM}

Suggested Fix

Ensure the IntersectionObserver is initialized before any register calls can be made. This can be achieved by lazily initializing the observer within the register function itself if it doesn't exist, or by altering the registration flow to ensure the useEffect hook in the provider has run first.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/dashboards/contexts/widgetSyncContext.tsx#L57-L72

Potential issue: A race condition exists between the chart component's `ref` callback
and the `WidgetSyncContextProvider`'s `useEffect` hook. React's lifecycle guarantees
that `ref` callbacks execute before `useEffect`. If a chart's `onChartReady` event fires
(triggering the `register` function) during the same render as the provider's first
render, `register` will be called before the `useEffect` hook has initialized the
`IntersectionObserver`. This results in the `observerRef.current?.observe(dom)` call
being a no-op, as `observerRef.current` is `null`. Consequently, the chart will never be
observed and will remain in the sync group permanently, even when scrolled out of view,
defeating the purpose of the optimization.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

• ✅ Fixed: Observer loses tracked charts on groupName re-render
  • Used useState for stable groupName and re-observe all charts when observer changes in useEffect.
• ✅ Fixed: Registered charts never cleaned up from map
  • Changed register to return an unregister function that removes charts from Map and calls unobserve.
• ✅ Fixed: Observer is null when child charts first register
  • Created observer synchronously using useMemo instead of useEffect so it exists before children mount.

Or push these changes by commenting:

@cursor push e106592da0

Preview (e106592da0)

diff --git a/static/app/views/dashboards/contexts/widgetSyncContext.tsx b/static/app/views/dashboards/contexts/widgetSyncContext.tsx
--- a/static/app/views/dashboards/contexts/widgetSyncContext.tsx
+++ b/static/app/views/dashboards/contexts/widgetSyncContext.tsx
@@ -1,12 +1,12 @@
 import type {ReactNode} from 'react';
-import {useCallback, useEffect, useRef} from 'react';
+import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import type {EChartsType} from 'echarts';
 import * as echarts from 'echarts';
 
 import {uniqueId} from 'sentry/utils/guid';
 import {createDefinedContext} from 'sentry/utils/performance/contexts/utils';
 
-type RegistrationFunction = (chart: EChartsType) => void;
+type RegistrationFunction = (chart: EChartsType) => () => void;
 
 interface WidgetSyncContext {
   groupName: string;
@@ -26,13 +26,17 @@
 
 export function WidgetSyncContextProvider({
   children,
-  groupName = uniqueId(),
+  groupName: groupNameProp,
 }: WidgetSyncContextProviderProps) {
+  // Use useState to ensure stable groupName when not provided as prop
+  const [stableGroupName] = useState(() => groupNameProp ?? uniqueId());
+  const groupName = groupNameProp ?? stableGroupName;
+
   const chartsRef = useRef<Map<Element, EChartsType>>(new Map());
-  const observerRef = useRef<IntersectionObserver | null>(null);
 
-  useEffect(() => {
-    observerRef.current = new IntersectionObserver(entries => {
+  // Create observer synchronously so it exists before children mount (fixes Bug 3)
+  const observerRef = useMemo(() => {
+    return new IntersectionObserver(entries => {
       for (const entry of entries) {
         const chart = chartsRef.current.get(entry.target);
         if (!chart) {
@@ -48,27 +52,41 @@
 
       echarts?.connect(groupName);
     });
+  }, [groupName]);
 
+  // Re-observe all existing charts when observer changes (fixes Bug 1)
+  useEffect(() => {
+    const charts = Array.from(chartsRef.current.entries());
+    for (const [dom] of charts) {
+      observerRef.observe(dom);
+    }
+
     return () => {
-      observerRef.current?.disconnect();
+      observerRef.disconnect();
     };
-  }, [groupName]);
+  }, [observerRef]);
 
   const register = useCallback(
     (chart: EChartsType) => {
       const dom = chart.getDom();
       if (!dom) {
-        return;
+        return () => {};
       }
 
       chartsRef.current.set(dom, chart);
-      observerRef.current?.observe(dom);
+      observerRef.observe(dom);
 
       // Set the group immediately for charts that may already be visible
       chart.group = groupName;
       echarts?.connect(groupName);
+
+      // Return unregister function for cleanup (fixes Bug 2)
+      return () => {
+        chartsRef.current.delete(dom);
+        observerRef.unobserve(dom);
+      };
     },
-    [groupName]
+    [groupName, observerRef]
   );
 
   return (
@@ -89,7 +107,7 @@
   if (!context) {
     // The provider was not registered, return a dummy function
     return {
-      register: (_p: any) => null,
+      register: (_p: any) => () => {},
       groupName: '',
     };
   }

diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx
--- a/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx
+++ b/static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx
@@ -1,4 +1,4 @@
-import {useCallback, useMemo, useRef, useState} from 'react';
+import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import {useTheme} from '@emotion/react';
 import {mergeRefs} from '@react-aria/utils';
 import * as Sentry from '@sentry/react';
@@ -137,6 +137,7 @@
 
   const chartRef = useRef<ReactEchartsRef | null>(null);
   const {register: registerWithWidgetSyncContext, groupName} = useWidgetSyncContext();
+  const unregisterRef = useRef<(() => void) | null>(null);
 
   const pageFilters = usePageFilters();
   const {start, end, period, utc} =
@@ -445,11 +446,18 @@
   const handleChartReady = useCallback(
     (instance: echarts.ECharts) => {
       onChartReadyZoom(instance);
-      registerWithWidgetSyncContext(instance);
+      unregisterRef.current = registerWithWidgetSyncContext(instance);
     },
     [onChartReadyZoom, registerWithWidgetSyncContext]
   );
 
+  // Cleanup: unregister chart on unmount
+  useEffect(() => {
+    return () => {
+      unregisterRef.current?.();
+    };
+  }, []);
+
   const showXAxisProp = props.showXAxis ?? 'auto';
   const showXAxis = showXAxisProp === 'auto';

_{This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.}

[cursor]

Observer loses tracked charts on groupName re-render

Medium Severity

When the useEffect re-runs (due to groupName changing), the old IntersectionObserver is disconnected and a new one is created, but the new observer never re-observes the elements already stored in chartsRef.current. Since register is only called once per chart on mount, previously registered charts permanently lose visibility tracking. This is especially problematic for multiQueryMode/content.tsx, which uses WidgetSyncContextProvider without a groupName prop — meaning uniqueId() generates a new value on every render, triggering the effect repeatedly and breaking the optimization entirely.

Additional Locations (1)

• static/app/views/dashboards/contexts/widgetSyncContext.tsx#L28-L29

 

[cursor]

Registered charts never cleaned up from map

Low Severity

The register function adds chart DOM elements and echarts instances to chartsRef.current and observes them, but there's no corresponding unregister/cleanup. When individual chart widgets unmount, their entries remain in the Map indefinitely, holding references to detached DOM nodes and destroyed echarts instances and preventing garbage collection. This is a memory leak introduced by this PR since the old code had no persistent per-chart state.

 

[cursor]

Observer is null when child charts first register

High Severity

React runs child useEffects before parent useEffects. On initial mount, child charts fire onChartReady → register in their own effects, which runs before the parent WidgetSyncContextProvider's useEffect that creates the IntersectionObserver. At that point observerRef.current is still null, so observerRef.current?.observe(dom) is a no-op. The chart is added to chartsRef but never actually observed, meaning the visibility-based optimization — the entire purpose of this PR — silently never activates for any initially-mounted chart.

Additional Locations (1)

• static/app/views/dashboards/contexts/widgetSyncContext.tsx#L63-L65