From d8fe731255c9b7749b10725e7705ffb6f9f15007 Mon Sep 17 00:00:00 2001 From: ryan953 <187460+ryan953@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:33:22 +0000 Subject: [PATCH 1/3] ref(discover): Convert ResultsChart components to functional Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> --- .../views/discover/results/resultsChart.tsx | 411 ++++++++---------- 1 file changed, 189 insertions(+), 222 deletions(-) diff --git a/static/app/views/discover/results/resultsChart.tsx b/static/app/views/discover/results/resultsChart.tsx index f4ae29e6b7e8..9060daf32c89 100644 --- a/static/app/views/discover/results/resultsChart.tsx +++ b/static/app/views/discover/results/resultsChart.tsx @@ -1,10 +1,8 @@ -import {Component, Fragment} from 'react'; +import {Fragment, memo, useContext, useMemo} from 'react'; import styled from '@emotion/styled'; import type {Location} from 'history'; import isEqual from 'lodash/isEqual'; -import type {SelectValue} from '@sentry/scraps/select'; - import type {Client} from 'sentry/api'; import {AreaChart} from 'sentry/components/charts/areaChart'; import {BarChart} from 'sentry/components/charts/barChart'; @@ -27,9 +25,8 @@ import { TOP_N, } from 'sentry/utils/discover/types'; import {getDynamicText} from 'sentry/utils/getDynamicText'; -import {valueIsEqual} from 'sentry/utils/object/valueIsEqual'; import {decodeScalar} from 'sentry/utils/queryString'; -import {withApi} from 'sentry/utils/withApi'; +import {useApi} from 'sentry/utils/useApi'; import {isCustomMeasurement} from 'sentry/views/dashboards/utils'; import {ChartFooter} from 'sentry/views/discover/results/chartFooter'; @@ -43,122 +40,114 @@ type ResultsChartProps = { customMeasurements?: CustomMeasurementCollection | undefined; }; -class ResultsChart extends Component { - shouldComponentUpdate(nextProps: ResultsChartProps) { - const {eventView, ...restProps} = this.props; - const {eventView: nextEventView, ...restNextProps} = nextProps; - - if (!eventView.isEqualTo(nextEventView)) { - return true; - } - - return !isEqual(restProps, restNextProps); - } - - render() { - const { - api, - eventView, - location, - organization, - confirmedQuery, - yAxisValue, - customMeasurements, - } = this.props; +const ResultsChart = memo(function ResultsChart({ + api, + eventView, + location, + organization, + confirmedQuery, + yAxisValue, + customMeasurements, +}: ResultsChartProps) { + const globalSelection = eventView.getPageFilters(); + const start = globalSelection.datetime.start + ? getUtcToLocalDateObject(globalSelection.datetime.start) + : null; - const globalSelection = eventView.getPageFilters(); - const start = globalSelection.datetime.start - ? getUtcToLocalDateObject(globalSelection.datetime.start) - : null; + const end = globalSelection.datetime.end + ? getUtcToLocalDateObject(globalSelection.datetime.end) + : null; - const end = globalSelection.datetime.end - ? getUtcToLocalDateObject(globalSelection.datetime.end) - : null; - - const {utc} = normalizeDateTimeParams(location.query); - const apiPayload = eventView.getEventsAPIPayload(location); - const display = eventView.getDisplayMode(); - const isTopEvents = - display === DisplayModes.TOP5 || display === DisplayModes.DAILYTOP5; - const isPeriod = display === DisplayModes.DEFAULT || display === DisplayModes.TOP5; - const isDaily = display === DisplayModes.DAILYTOP5 || display === DisplayModes.DAILY; - const isPrevious = display === DisplayModes.PREVIOUS; - const referrer = `api.discover.${display}-chart`; - const topEvents = eventView.topEvents ? parseInt(eventView.topEvents, 10) : TOP_N; - const aggregateParam = getAggregateArg(yAxisValue[0]!) || ''; - const customPerformanceMetricFieldType = isCustomMeasurement(aggregateParam) - ? customMeasurements - ? customMeasurements[aggregateParam]?.fieldType - : null - : null; - const chartComponent = - display === DisplayModes.BAR - ? BarChart - : display === DisplayModes.PREVIOUS + const {utc} = normalizeDateTimeParams(location.query); + const apiPayload = eventView.getEventsAPIPayload(location); + const display = eventView.getDisplayMode(); + const isTopEvents = + display === DisplayModes.TOP5 || display === DisplayModes.DAILYTOP5; + const isPeriod = display === DisplayModes.DEFAULT || display === DisplayModes.TOP5; + const isDaily = display === DisplayModes.DAILYTOP5 || display === DisplayModes.DAILY; + const isPrevious = display === DisplayModes.PREVIOUS; + const referrer = `api.discover.${display}-chart`; + const topEvents = eventView.topEvents ? parseInt(eventView.topEvents, 10) : TOP_N; + const aggregateParam = getAggregateArg(yAxisValue[0]!) || ''; + const customPerformanceMetricFieldType = isCustomMeasurement(aggregateParam) + ? customMeasurements + ? customMeasurements[aggregateParam]?.fieldType + : null + : null; + const chartComponent = + display === DisplayModes.BAR + ? BarChart + : display === DisplayModes.PREVIOUS + ? AreaChart + : customPerformanceMetricFieldType === 'size' && isTopEvents ? AreaChart - : customPerformanceMetricFieldType === 'size' && isTopEvents - ? AreaChart - : undefined; - const interval = - display === DisplayModes.BAR - ? getInterval( - { - start, - end, - period: globalSelection.datetime.period, - utc: utc === 'true', - }, - 'low' - ) - : eventView.interval; + : undefined; + const interval = + display === DisplayModes.BAR + ? getInterval( + { + start, + end, + period: globalSelection.datetime.period, + utc: utc === 'true', + }, + 'low' + ) + : eventView.interval; - const seriesLabels = yAxisValue.map(stripEquationPrefix); - const disableableSeries = [ - ...seriesLabels, - ...seriesLabels.map(getPreviousSeriesName), - ]; + const seriesLabels = yAxisValue.map(stripEquationPrefix); + const disableableSeries = [ + ...seriesLabels, + ...seriesLabels.map(getPreviousSeriesName), + ]; - return ( - - {getDynamicText({ - value: ( - - ), - fixed: , - })} - - ); + return ( + + {getDynamicText({ + value: ( + + ), + fixed: , + })} + + ); +}, +function areEqual(prev: ResultsChartProps, next: ResultsChartProps) { + const {eventView, ...restPrev} = prev; + const {eventView: nextEventView, ...restNext} = next; + if (!eventView.isEqualTo(nextEventView)) { + return false; } -} + return isEqual(restPrev, restNext); +}); type ContainerProps = { - api: Client; confirmedQuery: boolean; eventView: EventView; location: Location; @@ -173,123 +162,101 @@ type ContainerProps = { yAxis: string[]; }; -type ContainerState = { - yAxisOptions: Array>; -}; - -class ResultsChartContainer extends Component { - state: ContainerState = { - yAxisOptions: this.props.eventView.getYAxisOptions(), - }; +const ResultsChartContainer = memo(function ResultsChartContainer({ + eventView, + location, + total, + onAxisChange, + onDisplayChange, + onIntervalChange, + onTopEventsChange, + organization, + confirmedQuery, + yAxis, +}: ContainerProps) { + const api = useApi(); + const customMeasurementsContext = useContext(CustomMeasurementsContext); - UNSAFE_componentWillReceiveProps(nextProps: any) { - const yAxisOptions = this.props.eventView.getYAxisOptions(); - const nextYAxisOptions = nextProps.eventView.getYAxisOptions(); + const yAxisOptions = useMemo( + () => eventView.getYAxisOptions(), + // eslint-disable-next-line react-hooks/exhaustive-deps + [eventView] + ); - if (!valueIsEqual(yAxisOptions, nextYAxisOptions, true)) { - this.setState({yAxisOptions: nextYAxisOptions}); - } - } - - shouldComponentUpdate(nextProps: ContainerProps) { - const {eventView, ...restProps} = this.props; - const {eventView: nextEventView, ...restNextProps} = nextProps; - - if ( - !eventView.isEqualTo(nextEventView) || - this.props.confirmedQuery !== nextProps.confirmedQuery - ) { + const hasQueryFeature = organization.features.includes('discover-query'); + const displayOptions = eventView + .getDisplayOptions() + .filter(opt => { + // top5 modes are only available with larger packages in saas. + // We remove instead of disable here as showing tooltips in dropdown + // menus is clunky. + if (TOP_EVENT_MODES.includes(opt.value) && !hasQueryFeature) { + return false; + } return true; - } - - return !isEqual(restProps, restNextProps); - } - - render() { - const { - api, - eventView, - location, - total, - onAxisChange, - onDisplayChange, - onIntervalChange, - onTopEventsChange, - organization, - confirmedQuery, - yAxis, - } = this.props; - - const {yAxisOptions} = this.state; - - const hasQueryFeature = organization.features.includes('discover-query'); - const displayOptions = eventView - .getDisplayOptions() - .filter(opt => { - // top5 modes are only available with larger packages in saas. - // We remove instead of disable here as showing tooltips in dropdown - // menus is clunky. - if (TOP_EVENT_MODES.includes(opt.value) && !hasQueryFeature) { - return false; - } - return true; - }) - .map(opt => { - // Can only use default display or total daily with multi y axis - if (TOP_EVENT_MODES.includes(opt.value)) { - opt.label = DisplayModes.TOP5 === opt.value ? 'Top Period' : 'Top Daily'; - } - if ( - yAxis.length > 1 && - !MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(opt.value as DisplayModes) - ) { - return { - ...opt, - disabled: true, - tooltip: t( - 'Change the Y-Axis dropdown to display only 1 function to use this view.' - ), - }; - } - return opt; - }); + }) + .map(opt => { + // Can only use default display or total daily with multi y axis + if (TOP_EVENT_MODES.includes(opt.value)) { + opt.label = DisplayModes.TOP5 === opt.value ? 'Top Period' : 'Top Daily'; + } + if ( + yAxis.length > 1 && + !MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(opt.value as DisplayModes) + ) { + return { + ...opt, + disabled: true, + tooltip: t( + 'Change the Y-Axis dropdown to display only 1 function to use this view.' + ), + }; + } + return opt; + }); - return ( - - {(yAxis.length > 0 && ( - - {contextValue => ( - - )} - - )) || {t('No Y-Axis selected.')}} - + {(yAxis.length > 0 && ( + - - ); + )) || {t('No Y-Axis selected.')}} + + + ); +}, +function areContainerEqual(prev: ContainerProps, next: ContainerProps) { + const {eventView, ...restPrev} = prev; + const {eventView: nextEventView, ...restNext} = next; + if ( + !eventView.isEqualTo(nextEventView) || + prev.confirmedQuery !== next.confirmedQuery + ) { + return false; } -} + return isEqual(restPrev, restNext); +}); -export default withApi(ResultsChartContainer); +export default ResultsChartContainer; const StyledPanel = styled(Panel)` @container (min-width: ${p => p.theme.container['4xl']}) { From 33ecb006d51a4ad06be5a54f58d6cd72a33e385e Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:34:16 +0000 Subject: [PATCH 2/3] :hammer_and_wrench: apply pre-commit fixes --- .../views/discover/results/resultsChart.tsx | 380 +++++++++--------- 1 file changed, 191 insertions(+), 189 deletions(-) diff --git a/static/app/views/discover/results/resultsChart.tsx b/static/app/views/discover/results/resultsChart.tsx index 9060daf32c89..f454ba377538 100644 --- a/static/app/views/discover/results/resultsChart.tsx +++ b/static/app/views/discover/results/resultsChart.tsx @@ -40,112 +40,114 @@ type ResultsChartProps = { customMeasurements?: CustomMeasurementCollection | undefined; }; -const ResultsChart = memo(function ResultsChart({ - api, - eventView, - location, - organization, - confirmedQuery, - yAxisValue, - customMeasurements, -}: ResultsChartProps) { - const globalSelection = eventView.getPageFilters(); - const start = globalSelection.datetime.start - ? getUtcToLocalDateObject(globalSelection.datetime.start) - : null; +const ResultsChart = memo( + function ResultsChart({ + api, + eventView, + location, + organization, + confirmedQuery, + yAxisValue, + customMeasurements, + }: ResultsChartProps) { + const globalSelection = eventView.getPageFilters(); + const start = globalSelection.datetime.start + ? getUtcToLocalDateObject(globalSelection.datetime.start) + : null; - const end = globalSelection.datetime.end - ? getUtcToLocalDateObject(globalSelection.datetime.end) - : null; + const end = globalSelection.datetime.end + ? getUtcToLocalDateObject(globalSelection.datetime.end) + : null; - const {utc} = normalizeDateTimeParams(location.query); - const apiPayload = eventView.getEventsAPIPayload(location); - const display = eventView.getDisplayMode(); - const isTopEvents = - display === DisplayModes.TOP5 || display === DisplayModes.DAILYTOP5; - const isPeriod = display === DisplayModes.DEFAULT || display === DisplayModes.TOP5; - const isDaily = display === DisplayModes.DAILYTOP5 || display === DisplayModes.DAILY; - const isPrevious = display === DisplayModes.PREVIOUS; - const referrer = `api.discover.${display}-chart`; - const topEvents = eventView.topEvents ? parseInt(eventView.topEvents, 10) : TOP_N; - const aggregateParam = getAggregateArg(yAxisValue[0]!) || ''; - const customPerformanceMetricFieldType = isCustomMeasurement(aggregateParam) - ? customMeasurements - ? customMeasurements[aggregateParam]?.fieldType - : null - : null; - const chartComponent = - display === DisplayModes.BAR - ? BarChart - : display === DisplayModes.PREVIOUS - ? AreaChart - : customPerformanceMetricFieldType === 'size' && isTopEvents + const {utc} = normalizeDateTimeParams(location.query); + const apiPayload = eventView.getEventsAPIPayload(location); + const display = eventView.getDisplayMode(); + const isTopEvents = + display === DisplayModes.TOP5 || display === DisplayModes.DAILYTOP5; + const isPeriod = display === DisplayModes.DEFAULT || display === DisplayModes.TOP5; + const isDaily = display === DisplayModes.DAILYTOP5 || display === DisplayModes.DAILY; + const isPrevious = display === DisplayModes.PREVIOUS; + const referrer = `api.discover.${display}-chart`; + const topEvents = eventView.topEvents ? parseInt(eventView.topEvents, 10) : TOP_N; + const aggregateParam = getAggregateArg(yAxisValue[0]!) || ''; + const customPerformanceMetricFieldType = isCustomMeasurement(aggregateParam) + ? customMeasurements + ? customMeasurements[aggregateParam]?.fieldType + : null + : null; + const chartComponent = + display === DisplayModes.BAR + ? BarChart + : display === DisplayModes.PREVIOUS ? AreaChart - : undefined; - const interval = - display === DisplayModes.BAR - ? getInterval( - { - start, - end, - period: globalSelection.datetime.period, - utc: utc === 'true', - }, - 'low' - ) - : eventView.interval; + : customPerformanceMetricFieldType === 'size' && isTopEvents + ? AreaChart + : undefined; + const interval = + display === DisplayModes.BAR + ? getInterval( + { + start, + end, + period: globalSelection.datetime.period, + utc: utc === 'true', + }, + 'low' + ) + : eventView.interval; - const seriesLabels = yAxisValue.map(stripEquationPrefix); - const disableableSeries = [ - ...seriesLabels, - ...seriesLabels.map(getPreviousSeriesName), - ]; + const seriesLabels = yAxisValue.map(stripEquationPrefix); + const disableableSeries = [ + ...seriesLabels, + ...seriesLabels.map(getPreviousSeriesName), + ]; - return ( - - {getDynamicText({ - value: ( - - ), - fixed: , - })} - - ); -}, -function areEqual(prev: ResultsChartProps, next: ResultsChartProps) { - const {eventView, ...restPrev} = prev; - const {eventView: nextEventView, ...restNext} = next; - if (!eventView.isEqualTo(nextEventView)) { - return false; + return ( + + {getDynamicText({ + value: ( + + ), + fixed: , + })} + + ); + }, + function areEqual(prev: ResultsChartProps, next: ResultsChartProps) { + const {eventView, ...restPrev} = prev; + const {eventView: nextEventView, ...restNext} = next; + if (!eventView.isEqualTo(nextEventView)) { + return false; + } + return isEqual(restPrev, restNext); } - return isEqual(restPrev, restNext); -}); +); type ContainerProps = { confirmedQuery: boolean; @@ -162,101 +164,101 @@ type ContainerProps = { yAxis: string[]; }; -const ResultsChartContainer = memo(function ResultsChartContainer({ - eventView, - location, - total, - onAxisChange, - onDisplayChange, - onIntervalChange, - onTopEventsChange, - organization, - confirmedQuery, - yAxis, -}: ContainerProps) { - const api = useApi(); - const customMeasurementsContext = useContext(CustomMeasurementsContext); +export const ResultsChartContainer = memo( + function ResultsChartContainer({ + eventView, + location, + total, + onAxisChange, + onDisplayChange, + onIntervalChange, + onTopEventsChange, + organization, + confirmedQuery, + yAxis, + }: ContainerProps) { + const api = useApi(); + const customMeasurementsContext = useContext(CustomMeasurementsContext); - const yAxisOptions = useMemo( - () => eventView.getYAxisOptions(), - // eslint-disable-next-line react-hooks/exhaustive-deps - [eventView] - ); + const yAxisOptions = useMemo( + () => eventView.getYAxisOptions(), + // eslint-disable-next-line react-hooks/exhaustive-deps + [eventView] + ); - const hasQueryFeature = organization.features.includes('discover-query'); - const displayOptions = eventView - .getDisplayOptions() - .filter(opt => { - // top5 modes are only available with larger packages in saas. - // We remove instead of disable here as showing tooltips in dropdown - // menus is clunky. - if (TOP_EVENT_MODES.includes(opt.value) && !hasQueryFeature) { - return false; - } - return true; - }) - .map(opt => { - // Can only use default display or total daily with multi y axis - if (TOP_EVENT_MODES.includes(opt.value)) { - opt.label = DisplayModes.TOP5 === opt.value ? 'Top Period' : 'Top Daily'; - } - if ( - yAxis.length > 1 && - !MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(opt.value as DisplayModes) - ) { - return { - ...opt, - disabled: true, - tooltip: t( - 'Change the Y-Axis dropdown to display only 1 function to use this view.' - ), - }; - } - return opt; - }); + const hasQueryFeature = organization.features.includes('discover-query'); + const displayOptions = eventView + .getDisplayOptions() + .filter(opt => { + // top5 modes are only available with larger packages in saas. + // We remove instead of disable here as showing tooltips in dropdown + // menus is clunky. + if (TOP_EVENT_MODES.includes(opt.value) && !hasQueryFeature) { + return false; + } + return true; + }) + .map(opt => { + // Can only use default display or total daily with multi y axis + if (TOP_EVENT_MODES.includes(opt.value)) { + opt.label = DisplayModes.TOP5 === opt.value ? 'Top Period' : 'Top Daily'; + } + if ( + yAxis.length > 1 && + !MULTI_Y_AXIS_SUPPORTED_DISPLAY_MODES.includes(opt.value as DisplayModes) + ) { + return { + ...opt, + disabled: true, + tooltip: t( + 'Change the Y-Axis dropdown to display only 1 function to use this view.' + ), + }; + } + return opt; + }); - return ( - - {(yAxis.length > 0 && ( - + {(yAxis.length > 0 && ( + + )) || {t('No Y-Axis selected.')}} + - )) || {t('No Y-Axis selected.')}} - - - ); -}, -function areContainerEqual(prev: ContainerProps, next: ContainerProps) { - const {eventView, ...restPrev} = prev; - const {eventView: nextEventView, ...restNext} = next; - if ( - !eventView.isEqualTo(nextEventView) || - prev.confirmedQuery !== next.confirmedQuery - ) { - return false; + + ); + }, + function areContainerEqual(prev: ContainerProps, next: ContainerProps) { + const {eventView, ...restPrev} = prev; + const {eventView: nextEventView, ...restNext} = next; + if ( + !eventView.isEqualTo(nextEventView) || + prev.confirmedQuery !== next.confirmedQuery + ) { + return false; + } + return isEqual(restPrev, restNext); } - return isEqual(restPrev, restNext); -}); - -export default ResultsChartContainer; +); const StyledPanel = styled(Panel)` @container (min-width: ${p => p.theme.container['4xl']}) { From fa28391c895f113365181e8d6fe36064cf5e64ab Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Wed, 23 Sep 2026 15:00:26 -0700 Subject: [PATCH 3/3] fix(discover): Update ResultsChart importers for named export ResultsChartContainer is now a named export that gets the api client from useApi, so callers must use the named import and stop passing api. Also drop the eslint-disable on the useMemo, since its deps are complete and oxlint reports the directive as unused. --- static/app/views/discover/results.tsx | 8 +++----- static/app/views/discover/results/resultsChart.spec.tsx | 6 +++--- static/app/views/discover/results/resultsChart.tsx | 6 +----- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/static/app/views/discover/results.tsx b/static/app/views/discover/results.tsx index eb341a75429c..bcdbd1f9cf2a 100644 --- a/static/app/views/discover/results.tsx +++ b/static/app/views/discover/results.tsx @@ -83,7 +83,7 @@ import { DEFAULT_EVENT_VIEW, DEFAULT_EVENT_VIEW_MAP, } from 'sentry/views/discover/results/data'; -import ResultsChart from 'sentry/views/discover/results/resultsChart'; +import {ResultsChartContainer} from 'sentry/views/discover/results/resultsChart'; import {ResultsHeader} from 'sentry/views/discover/results/resultsHeader'; import {ResultsSearchQueryBuilder} from 'sentry/views/discover/results/resultsSearchQueryBuilder'; import {SampleDataAlert} from 'sentry/views/discover/results/sampleDataAlert'; @@ -647,8 +647,7 @@ export class Results extends Component { }; render() { - const {organization, location, selection, api, setSavedQuery, isHomepage} = - this.props; + const {organization, location, selection, setSavedQuery, isHomepage} = this.props; const { eventView, error, @@ -745,8 +744,7 @@ export class Results extends Component { organization={organization} location={location} > - ResultsChart', () => { const features = ['discover-basic']; @@ -32,7 +32,7 @@ describe('Discover > ResultsChart', () => { it('only allows default, daily, previous period, and bar display modes when multiple y axis are selected', async () => { render( - ResultsChart', () => { it('does not display a chart if no y axis is selected', async () => { render( - eventView.getYAxisOptions(), - // eslint-disable-next-line react-hooks/exhaustive-deps - [eventView] - ); + const yAxisOptions = useMemo(() => eventView.getYAxisOptions(), [eventView]); const hasQueryFeature = organization.features.includes('discover-query'); const displayOptions = eventView