fix(perf): Landing issues errors widget switch (#29883)

* fix(perf): Landing issues errors widget switch

Switching between issues and errors was causing issues because of leftover widget data from the previous request. This PR tags widgetData into the state based on the current chart setting, which stops data from being leaked, and additionally adds a wrapping component around each query to perform cleanup on unmounted query components (due to enabled changing when a dependent query can't be called).

static/app/views/performance/landing/widgets/components/performanceWidget.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useCallback, useState} from 'react';
+import {Fragment, useCallback, useRef, useState} from 'react';
 import {withRouter} from 'react-router';
 import styled from '@emotion/styled';
 
@@ -25,21 +25,31 @@ import {WidgetHeader} from './widgetHeader';
 export function GenericPerformanceWidget<T extends WidgetDataConstraint>(
   props: WidgetPropUnion<T>
 ) {
-  const [widgetData, setWidgetData] = useState<T>({} as T);
-  const [nextWidgetData, setNextWidgetData] = useState<T>({} as T);
+  // Use object keyed to chart setting so switching between charts of a similar type doesn't retain data with query components still having inflight requests.
+  const [allWidgetData, setWidgetData] = useState<{[chartSetting: string]: T}>({});
+  const widgetData = allWidgetData[props.chartSetting] ?? {};
+  const widgetDataRef = useRef(widgetData);
 
   const setWidgetDataForKey = useCallback(
     (dataKey: string, result?: WidgetDataResult) => {
-      if (result) {
-        setNextWidgetData({...nextWidgetData, [dataKey]: result});
-      }
-      if (result?.hasData || result?.isErrored) {
-        setWidgetData({...widgetData, [dataKey]: result});
-      }
+      const _widgetData = widgetDataRef.current;
+      const newWidgetData = {..._widgetData, [dataKey]: result};
+      widgetDataRef.current = newWidgetData;
+      setWidgetData({[props.chartSetting]: newWidgetData});
     },
-    [widgetData, nextWidgetData, setWidgetData, setNextWidgetData]
+    [allWidgetData, setWidgetData]
   );
-  const widgetProps = {widgetData, nextWidgetData, setWidgetDataForKey};
+  const removeWidgetDataForKey = useCallback(
+    (dataKey: string) => {
+      const _widgetData = widgetDataRef.current;
+      const newWidgetData = {..._widgetData};
+      delete newWidgetData[dataKey];
+      widgetDataRef.current = newWidgetData;
+      setWidgetData({[props.chartSetting]: newWidgetData});
+    },
+    [allWidgetData, setWidgetData]
+  );
+  const widgetProps = {widgetData, setWidgetDataForKey, removeWidgetDataForKey};
 
   const queries = Object.entries(props.Queries).map(([key, definition]) => ({
     ...definition,
@@ -55,6 +65,7 @@ export function GenericPerformanceWidget<T extends WidgetDataConstraint>(
       <QueryHandler
         widgetData={widgetData}
         setWidgetDataForKey={setWidgetDataForKey}
+        removeWidgetDataForKey={removeWidgetDataForKey}
         queryProps={props}
         queries={queries}
         api={api}
@@ -65,8 +76,7 @@ export function GenericPerformanceWidget<T extends WidgetDataConstraint>(
 }
 
 function _DataDisplay<T extends WidgetDataConstraint>(
-  props: GenericPerformanceWidgetProps<T> &
-    WidgetDataProps<T> & {nextWidgetData: T; totalHeight: number}
+  props: GenericPerformanceWidgetProps<T> & WidgetDataProps<T> & {totalHeight: number}
 ) {
   const {Visualizations, chartHeight, totalHeight, containerType, EmptyComponent} = props;
 
@@ -76,12 +86,9 @@ function _DataDisplay<T extends WidgetDataConstraint>(
 
   const numberKeys = Object.keys(props.Queries).length;
   const missingDataKeys = Object.values(props.widgetData).length !== numberKeys;
-  const missingNextDataKeys = Object.values(props.nextWidgetData).length !== numberKeys;
   const hasData =
     !missingDataKeys && Object.values(props.widgetData).every(d => !d || d.hasData);
-  const isLoading =
-    !missingNextDataKeys &&
-    Object.values(props.nextWidgetData).some(d => !d || d.isLoading);
+  const isLoading = Object.values(props.widgetData).some(d => !d || d.isLoading);
   const isErrored =
     !missingDataKeys && Object.values(props.widgetData).some(d => d && d.isErrored);
 

static/app/views/performance/landing/widgets/components/queryHandler.tsx

@@ -11,10 +11,26 @@ export function QueryHandler<T extends WidgetDataConstraint>(
   props: QueryHandlerProps<T>
 ) {
   const children = props.children ?? <Fragment />;
+
   if (!props.queries.length) {
     return <Fragment>{children}</Fragment>;
   }
 
+  return (
+    <Fragment>
+      {props.queries
+        .filter(q => (q.enabled ? q.enabled(props.widgetData) : true))
+        .map(query => (
+          <SingleQueryHandler key={query.queryKey} {...props} query={query} />
+        ))}
+    </Fragment>
+  );
+}
+
+function SingleQueryHandler<T extends WidgetDataConstraint>(
+  props: QueryHandlerProps<T> & {query: QueryDefinitionWithKey<T>}
+) {
+  const query = props.query;
   const globalSelection = props.queryProps.eventView.getGlobalSelection();
   const start = globalSelection.datetime.start
     ? getUtcToLocalDateObject(globalSelection.datetime.start)
@@ -24,35 +40,37 @@ export function QueryHandler<T extends WidgetDataConstraint>(
     ? getUtcToLocalDateObject(globalSelection.datetime.end)
     : null;
 
+  useEffect(
+    () => () => {
+      // Destroy previous data on unmount, in case enabled value changes and unmounts the query component.
+      props.removeWidgetDataForKey(query.queryKey);
+    },
+    []
+  );
+
   return (
-    <Fragment>
-      {props.queries
-        .filter(q => (q.enabled ? q.enabled(props.widgetData) : true))
-        .map(query => (
-          <query.component
-            key={query.queryKey}
-            fields={query.fields}
-            yAxis={query.fields}
-            start={start}
-            end={end}
-            period={globalSelection.datetime.period}
-            project={globalSelection.projects}
-            environment={globalSelection.environments}
-            organization={props.queryProps.organization}
-            orgSlug={props.queryProps.organization.slug}
-            query={props.queryProps.eventView.getQueryWithAdditionalConditions()}
-            widgetData={props.widgetData}
-          >
-            {results => {
-              return (
-                <Fragment>
-                  <QueryResultSaver<T> results={results} {...props} query={query} />
-                </Fragment>
-              );
-            }}
-          </query.component>
-        ))}
-    </Fragment>
+    <query.component
+      key={query.queryKey}
+      fields={query.fields}
+      yAxis={query.fields}
+      start={start}
+      end={end}
+      period={globalSelection.datetime.period}
+      project={globalSelection.projects}
+      environment={globalSelection.environments}
+      organization={props.queryProps.organization}
+      orgSlug={props.queryProps.organization.slug}
+      query={props.queryProps.eventView.getQueryWithAdditionalConditions()}
+      widgetData={props.widgetData}
+    >
+      {results => {
+        return (
+          <Fragment>
+            <QueryResultSaver<T> results={results} {...props} query={query} />
+          </Fragment>
+        );
+      }}
+    </query.component>
   );
 }
 

static/app/views/performance/landing/widgets/components/widgetContainer.tsx

@@ -1,4 +1,4 @@
-import {useState} from 'react';
+import {useEffect, useState} from 'react';
 import styled from '@emotion/styled';
 
 import MenuItem from 'app/components/menuItem';
@@ -109,6 +109,10 @@ const _WidgetContainer = (props: Props) => {
     setChartSettingState(setting);
   };
 
+  useEffect(() => {
+    setChartSettingState(_chartSetting);
+  }, [rest.defaultChartSetting]);
+
   const widgetProps = {
     chartSetting,
     ...WIDGET_DEFINITIONS({organization})[chartSetting],

static/app/views/performance/landing/widgets/types.tsx

@@ -8,6 +8,7 @@ import {DateString, Organization, OrganizationSummary} from 'app/types';
 import EventView from 'app/utils/discover/eventView';
 
 import {PerformanceWidgetContainerTypes} from './components/performanceWidgetContainer';
+import {PerformanceWidgetSetting} from './widgetDefinitions';
 
 export enum VisualizationDataState {
   ERROR = 'error',
@@ -97,6 +98,8 @@ type Subtitle<T> = FunctionComponent<{
 }>;
 
 export type GenericPerformanceWidgetProps<T extends WidgetDataConstraint> = {
+  chartSetting: PerformanceWidgetSetting;
+
   // Header;
   title: string;
   titleTooltip: string;
@@ -124,6 +127,7 @@ export type GenericPerformanceWithData<T extends WidgetDataConstraint> =
 export type WidgetDataProps<T> = {
   widgetData: T;
   setWidgetDataForKey: (dataKey: string, result?: WidgetDataResult) => void;
+  removeWidgetDataForKey: (dataKey: string) => void;
 };
 
 export type EventsRequestChildrenProps = RenderProps;

static/app/views/performance/landing/widgets/widgets/histogramWidget.tsx

@@ -12,8 +12,10 @@ import {Chart as HistogramChart} from 'app/views/performance/landing/chart/histo
 import {GenericPerformanceWidget} from '../components/performanceWidget';
 import {transformHistogramQuery} from '../transforms/transformHistogramQuery';
 import {WidgetDataResult} from '../types';
+import {PerformanceWidgetSetting} from '../widgetDefinitions';
 
 type Props = {
+  chartSetting: PerformanceWidgetSetting;
   title: string;
   titleTooltip: string;
   fields: string[];

static/app/views/performance/landing/widgets/widgets/singleFieldAreaWidget.tsx

@@ -16,8 +16,10 @@ import {GenericPerformanceWidget} from '../components/performanceWidget';
 import {transformEventsRequestToArea} from '../transforms/transformEventsToArea';
 import {QueryDefinition, WidgetDataResult} from '../types';
 import {eventsRequestQueryProps} from '../utils';
+import {PerformanceWidgetSetting} from '../widgetDefinitions';
 
 type Props = {
+  chartSetting: PerformanceWidgetSetting;
   title: string;
   titleTooltip: string;
   fields: string[];

static/app/views/performance/trends/chart.tsx

@@ -3,7 +3,6 @@ import {useTheme} from '@emotion/react';
 
 import ChartZoom from 'app/components/charts/chartZoom';
 import LineChart from 'app/components/charts/lineChart';
-import ReleaseSeries from 'app/components/charts/releaseSeries';
 import TransitionChart from 'app/components/charts/transitionChart';
 import TransparentLoadingMask from 'app/components/charts/transparentLoadingMask';
 import {getParams} from 'app/components/organizations/globalSelectionHeader/getParams';
@@ -236,13 +235,10 @@ export function Chart({
   trendChangeType,
   router,
   statsPeriod,
-  project,
-  environment,
   transaction,
   statsData,
   isLoading,
   location,
-  projects,
   start: propsStart,
   end: propsEnd,
   trendFunctionField,
@@ -311,11 +307,6 @@ export function Chart({
   const loading = isLoading;
   const reloading = isLoading;
 
-  const transactionProject = parseInt(
-    projects.find(({slug}) => transaction?.project === slug)?.id || '',
-    10
-  );
-
   const yMax = Math.max(
     maxValue,
     transaction?.aggregate_range_2 || 0,
@@ -329,11 +320,6 @@ export function Chart({
   const yDiff = yMax - yMin;
   const yMargin = yDiff * 0.1;
 
-  const queryExtra = {
-    showTransactions: trendChangeType,
-    yAxis: 'countDuration',
-  };
-
   const chartOptions = {
     tooltip: {
       valueFormatter: (value, seriesName) => {
@@ -380,50 +366,37 @@ export function Chart({
         );
 
         return (
-          <ReleaseSeries
-            start={start}
-            end={end}
-            queryExtra={queryExtra}
-            period={statsPeriod}
-            utc={utc === 'true'}
-            projects={isNaN(transactionProject) ? project : [transactionProject]}
-            environments={environment}
-            memoized
-          >
-            {({releaseSeries}) => (
-              <TransitionChart loading={loading} reloading={reloading}>
-                <TransparentLoadingMask visible={reloading} />
-                {getDynamicText({
-                  value: (
-                    <LineChart
-                      height={height}
-                      {...zoomRenderProps}
-                      {...chartOptions}
-                      onLegendSelectChanged={handleLegendSelectChanged}
-                      series={[...smoothedSeries, ...releaseSeries, ...intervalSeries]}
-                      seriesOptions={{
-                        showSymbol: false,
-                      }}
-                      legend={legend}
-                      toolBox={{
-                        show: false,
-                      }}
-                      grid={
-                        grid ?? {
-                          left: '10px',
-                          right: '10px',
-                          top: '40px',
-                          bottom: '0px',
-                        }
-                      }
-                      xAxis={disableXAxis ? {show: false} : undefined}
-                    />
-                  ),
-                  fixed: 'Duration Chart',
-                })}
-              </TransitionChart>
-            )}
-          </ReleaseSeries>
+          <TransitionChart loading={loading} reloading={reloading}>
+            <TransparentLoadingMask visible={reloading} />
+            {getDynamicText({
+              value: (
+                <LineChart
+                  height={height}
+                  {...zoomRenderProps}
+                  {...chartOptions}
+                  onLegendSelectChanged={handleLegendSelectChanged}
+                  series={[...smoothedSeries, ...intervalSeries]}
+                  seriesOptions={{
+                    showSymbol: false,
+                  }}
+                  legend={legend}
+                  toolBox={{
+                    show: false,
+                  }}
+                  grid={
+                    grid ?? {
+                      left: '10px',
+                      right: '10px',
+                      top: '40px',
+                      bottom: '0px',
+                    }
+                  }
+                  xAxis={disableXAxis ? {show: false} : undefined}
+                />
+              ),
+              fixed: 'Duration Chart',
+            })}
+          </TransitionChart>
         );
       }}
     </ChartZoom>

tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx

@@ -36,21 +36,50 @@ const WrappedComponent = ({data, ...rest}) => {
   );
 };
 
+const issuesPredicate = (url, options) =>
+  url.includes('eventsv2') && options.query?.query.includes('error');
+
 describe('Performance > Widgets > WidgetContainer', function () {
   let eventStatsMock;
   let eventsV2Mock;
   let eventsTrendsStats;
+
+  let issuesListMock;
+
   beforeEach(function () {
     eventStatsMock = MockApiClient.addMockResponse({
       method: 'GET',
       url: `/organizations/org-slug/events-stats/`,
       body: [],
     });
-    eventsV2Mock = MockApiClient.addMockResponse({
-      method: 'GET',
-      url: `/organizations/org-slug/eventsv2/`,
-      body: [],
-    });
+    eventsV2Mock = MockApiClient.addMockResponse(
+      {
+        method: 'GET',
+        url: `/organizations/org-slug/eventsv2/`,
+        body: [],
+      },
+      {predicate: (...args) => !issuesPredicate(...args)}
+    );
+    issuesListMock = MockApiClient.addMockResponse(
+      {
+        method: 'GET',
+        url: `/organizations/org-slug/eventsv2/`,
+        body: {
+          data: [
+            {
+              'issue.id': 123,
+              transaction: '/issue/:id/',
+              title: 'Error: Something is broken.',
+              'project.id': 1,
+              count: 3100,
+              issue: 'JAVASCRIPT-ABCD',
+            },
+          ],
+        },
+      },
+      {predicate: (...args) => issuesPredicate(...args)}
+    );
+
     eventsTrendsStats = MockApiClient.addMockResponse({
       method: 'GET',
       url: '/organizations/org-slug/events-trends-stats/',
@@ -288,8 +317,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
     expect(wrapper.find('div[data-test-id="performance-widget-title"]').text()).toEqual(
       'Most Related Issues'
     );
-    expect(eventsV2Mock).toHaveBeenCalledTimes(1);
-    expect(eventsV2Mock).toHaveBeenNthCalledWith(
+    expect(issuesListMock).toHaveBeenCalledTimes(1);
+    expect(issuesListMock).toHaveBeenNthCalledWith(
       1,
       expect.anything(),
       expect.objectContaining({
@@ -306,6 +335,38 @@ describe('Performance > Widgets > WidgetContainer', function () {
     );
   });
 
+  it('Switching from issues to errors widget', async function () {
+    const data = initializeData();
+
+    const wrapper = mountWithTheme(
+      <WrappedComponent
+        data={data}
+        defaultChartSetting={PerformanceWidgetSetting.MOST_RELATED_ISSUES}
+      />,
+      data.routerContext
+    );
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('div[data-test-id="performance-widget-title"]').text()).toEqual(
+      'Most Related Issues'
+    );
+    expect(issuesListMock).toHaveBeenCalledTimes(1);
+
+    wrapper.setProps({
+      defaultChartSetting: PerformanceWidgetSetting.MOST_RELATED_ERRORS,
+    });
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('div[data-test-id="performance-widget-title"]').text()).toEqual(
+      'Most Related Errors'
+    );
+    expect(eventsV2Mock).toHaveBeenCalledTimes(1);
+    expect(eventStatsMock).toHaveBeenCalledTimes(1);
+  });
+
   it('Most improved trends widget', async function () {
     const data = initializeData();