From fef8e0c642df33ca1a666f344e04400f8eed0ff0 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:22:35 +0000 Subject: [PATCH 1/9] fix(coding-conventions): Migrate ReleaseSeries class component to hook --- .../app/components/charts/releaseSeries.tsx | 473 ++++++++++-------- .../insights/common/components/chart.tsx | 35 +- .../charts/projectBaseSessionsChart.tsx | 94 ++-- 3 files changed, 321 insertions(+), 281 deletions(-) diff --git a/static/app/components/charts/releaseSeries.tsx b/static/app/components/charts/releaseSeries.tsx index 10aeadc54cb4..8dd2a7206f4e 100644 --- a/static/app/components/charts/releaseSeries.tsx +++ b/static/app/components/charts/releaseSeries.tsx @@ -1,8 +1,6 @@ -import {Component} from 'react'; -import type {Theme} from '@emotion/react'; -import {withTheme} from '@emotion/react'; -import type {Location, Query} from 'history'; -import isEqual from 'lodash/isEqual'; +import {useEffect, useRef, useState} from 'react'; +import {useTheme} from '@emotion/react'; +import type {Query} from 'history'; import memoize from 'lodash/memoize'; import partition from 'lodash/partition'; @@ -18,12 +16,11 @@ import {escape} from 'sentry/utils'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {getFormat, getFormattedDate, getUtcDateString} from 'sentry/utils/dates'; import {parseLinkHeader} from 'sentry/utils/parseLinkHeader'; +import useApi from 'sentry/utils/useApi'; import {useLocation} from 'sentry/utils/useLocation'; -import type {ReactRouter3Navigate} from 'sentry/utils/useNavigate'; import {useNavigate} from 'sentry/utils/useNavigate'; +import useOrganization from 'sentry/utils/useOrganization'; import {formatVersion} from 'sentry/utils/versions/formatVersion'; -import {withApi} from 'sentry/utils/withApi'; -import {withOrganization} from 'sentry/utils/withOrganization'; import {makeReleasesPathname} from 'sentry/views/explore/releases/utils/pathnames'; type ReleaseMetaBasic = { @@ -80,16 +77,11 @@ const getOrganizationReleasesMemoized = memoize( ); export interface ReleaseSeriesProps { - api: Client; - children: (s: State) => React.ReactNode; + children: (s: ReleaseSeriesState) => React.ReactNode; end: DateString; environments: readonly string[]; - location: Location; - navigate: ReactRouter3Navigate; - organization: Organization; projects: readonly number[]; start: DateString; - theme: Theme; emphasizeReleases?: string[]; enabled?: boolean; memoized?: boolean; @@ -102,251 +94,302 @@ export interface ReleaseSeriesProps { utc?: boolean | null; } -type State = { +type ReleaseSeriesState = { releaseSeries: Series[]; releases: ReleaseMetaBasic[] | null; }; +type UseReleaseSeriesProps = Omit; + /** - * @deprecated use useReleaseBubbles instead + * Hook that fetches releases and builds ECharts series data for release markers. */ -class ReleaseSeries extends Component { - state: State = { +export function useReleaseSeries({ + start, + end, + period, + environments, + projects, + query, + releases: propReleases, + enabled = true, + memoized, + emphasizeReleases, + preserveQueryParams, + queryExtra, + tooltip, + utc, +}: UseReleaseSeriesProps): ReleaseSeriesState { + const api = useApi(); + const organization = useOrganization(); + const theme = useTheme(); + const location = useLocation(); + const navigate = useNavigate(); + + // Keep utc in a ref so tooltip formatters always read the latest value + // without needing to be re-created (same pattern as original class used this.props.utc) + const utcRef = useRef(utc); + useEffect(() => { + utcRef.current = utc; + }, [utc]); + + const [state, setState] = useState({ releases: null, releaseSeries: [], - }; + }); - componentDidMount() { - this._isMounted = true; - const {releases, enabled = true} = this.props; + // Serialize non-primitive deps to stable strings so useEffect deps use + // value-equality instead of referential equality. The original class used + // lodash isEqual for these comparisons; callers like Discover rebuild Date + // objects and arrays on every render, so reference equality would re-fetch + // even when nothing logically changed. + const startKey = start ? getUtcDateString(start) : ''; + const endKey = end ? getUtcDateString(end) : ''; + const projectsKey = [...projects].join(','); + const environmentsKey = [...environments].join(','); + const emphasizeReleasesKey = emphasizeReleases?.join(',') ?? ''; - if (releases) { - // No need to fetch releases if passed in from props - this.setReleasesWithSeries(releases); - return; - } + // Stable refs for values used inside closures passed to echarts + const organizationRef = useRef(organization); + const locationRef = useRef(location); + const navigateRef = useRef(navigate); + const themeRef = useRef(theme); + const preserveQueryParamsRef = useRef(preserveQueryParams); + const queryExtraRef = useRef(queryExtra); + const tooltipRef = useRef(tooltip); + const environmentsRef = useRef(environments); + const startRef = useRef(start); + const endRef = useRef(end); + const periodRef = useRef(period); - if (enabled) { - this.fetchData(); - } - } + useEffect(() => { + organizationRef.current = organization; + locationRef.current = location; + navigateRef.current = navigate; + themeRef.current = theme; + preserveQueryParamsRef.current = preserveQueryParams; + queryExtraRef.current = queryExtra; + tooltipRef.current = tooltip; + environmentsRef.current = environments; + startRef.current = start; + endRef.current = end; + periodRef.current = period; + }); - componentDidUpdate(prevProps: any) { - const {enabled = true} = this.props; - - if ( - (!isEqual(prevProps.projects, this.props.projects) || - !isEqual(prevProps.environments, this.props.environments) || - !isEqual(prevProps.start, this.props.start) || - !isEqual(prevProps.end, this.props.end) || - !isEqual(prevProps.period, this.props.period) || - !isEqual(prevProps.query, this.props.query) || - (!prevProps.enabled && this.props.enabled)) && - enabled - ) { - this.fetchData(); - } else if (!isEqual(prevProps.emphasizeReleases, this.props.emphasizeReleases)) { - this.setReleasesWithSeries(this.state.releases); - } - } + function buildReleaseSeries(releases: ReleaseMetaBasic[]): Series[] { + const releaseSeries: Series[] = []; - componentWillUnmount() { - this._isMounted = false; - this.props.api.clear(); - } + function makeOneSeries(items: ReleaseMetaBasic[], lineStyle = {}): Series { + const org = organizationRef.current; + const loc = locationRef.current; + const nav = navigateRef.current; + const currentTheme = themeRef.current; + const currentPreserveQueryParams = preserveQueryParamsRef.current; + const currentQueryExtra = queryExtraRef.current; + const currentTooltip = tooltipRef.current; + const currentEnvironments = environmentsRef.current; + const currentStart = startRef.current; + const currentEnd = endRef.current; + const currentPeriod = periodRef.current; - _isMounted = false; - - async fetchData() { - const { - api, - organization, - projects, - environments, - period, - start, - end, - memoized, - query, - } = this.props; - const conditions: ReleaseConditions = { - start, - end, - project: projects, - environment: environments, - statsPeriod: period, - query, - }; - let hasMore = true; - const releases: ReleaseMetaBasic[] = []; - while (hasMore) { - try { - const getReleases = memoized - ? getOrganizationReleasesMemoized - : getOrganizationReleases; - const [newReleases, , resp] = await getReleases(api, organization, conditions); - releases.push(...newReleases); - if (this._isMounted) { - this.setReleasesWithSeries(releases); - } - - const pageLinks = resp?.getResponseHeader('Link'); - if (pageLinks) { - const paginationObject = parseLinkHeader(pageLinks); - hasMore = paginationObject?.next?.results ?? false; - conditions.cursor = paginationObject.next!.cursor; - } else { - hasMore = false; - } - } catch { - addErrorMessage(t('Error fetching releases')); - hasMore = false; + const extraQuery: Query = {...currentQueryExtra}; + extraQuery.project = loc.query.project; + if (currentPreserveQueryParams) { + extraQuery.environment = [...currentEnvironments]; + extraQuery.start = currentStart ? getUtcDateString(currentStart) : undefined; + extraQuery.end = currentEnd ? getUtcDateString(currentEnd) : undefined; + extraQuery.statsPeriod = currentPeriod || undefined; } - } - } - setReleasesWithSeries(releases: any) { - const {emphasizeReleases = []} = this.props; - const releaseSeries: Series[] = []; + const markLine = createMarkLine({ + animation: false, + lineStyle: { + color: currentTheme.tokens.dataviz.semantic.release, + opacity: 0.3, + type: 'solid', + ...lineStyle, + }, + label: { + show: false, + }, + data: items.map((release: ReleaseMetaBasic) => ({ + xAxis: +new Date(release.date), + name: formatVersion(release.version, true), + value: formatVersion(release.version, true), + + onClick: () => { + nav({ + pathname: makeReleasesPathname({ + organization: org, + path: `/${encodeURIComponent(release.version)}/`, + }), + query: extraQuery, + }); + }, - if (emphasizeReleases.length) { + label: { + formatter: () => formatVersion(release.version, true), + }, + })), + tooltip: currentTooltip || { + trigger: 'item', + formatter: ({data}: any) => { + // Should only happen when navigating pages + if (!data) { + return ''; + } + // Read utc from ref so formatter always gets the latest value + // even though this closure is long-lived inside echarts + const time = getFormattedDate( + data.value, + getFormat({timeZone: true, year: true}), + { + local: !utcRef.current, + } + ); + const version = escape(formatVersion(data.name, true)); + return [ + '
', + `
${t( + 'Release' + )} ${version}
`, + '
', + '
', + time, + '
', + '
', + ].join(''); + }, + }, + }); + + return { + id: 'release-lines', + seriesName: 'Releases', + color: currentTheme.tokens.dataviz.semantic.release, + data: [], + markLine, + }; + } + + if (emphasizeReleases?.length) { const [unemphasizedReleases, emphasizedReleases] = partition( releases, - release => !emphasizeReleases.includes(release.version) + release => !emphasizeReleases!.includes(release.version) ); if (unemphasizedReleases.length) { - releaseSeries.push(this.getReleaseSeries(unemphasizedReleases, {type: 'dotted'})); + releaseSeries.push(makeOneSeries(unemphasizedReleases, {type: 'dotted'})); } if (emphasizedReleases.length) { releaseSeries.push( - this.getReleaseSeries(emphasizedReleases, { + makeOneSeries(emphasizedReleases, { opacity: 0.8, }) ); } } else { - releaseSeries.push(this.getReleaseSeries(releases)); + releaseSeries.push(makeOneSeries(releases)); } - this.setState({ - releases, - releaseSeries, - }); + return releaseSeries; } - getReleaseSeries = (releases: any, lineStyle = {}) => { - const { - organization, - location, - navigate, - tooltip, - environments, - start, - end, - period, - preserveQueryParams, - queryExtra, - theme, - } = this.props; - - const query = {...queryExtra}; - query.project = location.query.project; - if (preserveQueryParams) { - query.environment = [...environments]; - query.start = start ? getUtcDateString(start) : undefined; - query.end = end ? getUtcDateString(end) : undefined; - query.statsPeriod = period || undefined; + useEffect(() => { + // If releases are passed directly via props, skip fetching + if (propReleases) { + setState({ + releases: propReleases, + releaseSeries: buildReleaseSeries(propReleases), + }); + return undefined; } - const markLine = createMarkLine({ - animation: false, - lineStyle: { - color: theme.tokens.dataviz.semantic.release, - opacity: 0.3, - type: 'solid', - ...lineStyle, - }, - label: { - show: false, - }, - data: releases.map((release: any) => ({ - xAxis: +new Date(release.date), - name: formatVersion(release.version, true), - value: formatVersion(release.version, true), - - onClick: () => { - navigate({ - pathname: makeReleasesPathname({ - organization, - path: `/${encodeURIComponent(release.version)}/`, - }), - query, - }); - }, + if (!enabled) { + return undefined; + } - label: { - formatter: () => formatVersion(release.version, true), - }, - })), - tooltip: tooltip || { - trigger: 'item', - formatter: ({data}: any) => { - // Should only happen when navigating pages - if (!data) { - return ''; + let cancelled = false; + + async function fetchData() { + const conditions: ReleaseConditions = { + start, + end, + project: projects, + environment: environments, + statsPeriod: period, + query, + }; + + let hasMore = true; + const releases: ReleaseMetaBasic[] = []; + while (hasMore) { + try { + const getReleases = memoized + ? getOrganizationReleasesMemoized + : getOrganizationReleases; + const [newReleases, , resp] = await getReleases(api, organization, conditions); + releases.push(...newReleases); + if (!cancelled) { + setState({ + releases, + releaseSeries: buildReleaseSeries(releases), + }); } - // XXX using this.props here as this function does not get re-run - // unless projects are changed. Using a closure variable would result - // in stale values. - const time = getFormattedDate( - data.value, - getFormat({timeZone: true, year: true}), - { - local: !this.props.utc, - } - ); - const version = escape(formatVersion(data.name, true)); - return [ - '
', - `
${t( - 'Release' - )} ${version}
`, - '
', - '
', - time, - '
', - '
', - ].join(''); - }, - }, - }); - return { - id: 'release-lines', - seriesName: 'Releases', - color: theme.tokens.dataviz.semantic.release, - data: [], - markLine, - }; - }; + const pageLinks = resp?.getResponseHeader('Link'); + if (pageLinks) { + const paginationObject = parseLinkHeader(pageLinks); + hasMore = paginationObject?.next?.results ?? false; + conditions.cursor = paginationObject.next!.cursor; + } else { + hasMore = false; + } + } catch { + addErrorMessage(t('Error fetching releases')); + hasMore = false; + } + } + } + + fetchData(); - render() { - const {children, enabled = true} = this.props; + return () => { + cancelled = true; + api.clear(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [startKey, endKey, period, projectsKey, environmentsKey, query, propReleases, enabled, memoized]); - return children({ - releases: enabled ? this.state.releases : [], - releaseSeries: enabled ? this.state.releaseSeries : [], + // Rebuild series when emphasizeReleases changes without re-fetching. + // We use the serialized key (not the array reference) so this effect only + // fires when the actual contents change, not on every render. + useEffect(() => { + setState(prev => { + if (prev.releases === null) { + return prev; + } + return { + ...prev, + releaseSeries: buildReleaseSeries(prev.releases), + }; }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [emphasizeReleasesKey]); + + if (!enabled) { + return {releases: [], releaseSeries: []}; } -} -function WithRouter(props: Omit) { - const location = useLocation(); - const navigate = useNavigate(); - return ; + return state; } /** - * @deprecated use useReleaseBubbles instead + * Render-prop component backed by useReleaseSeries. Prefer the hook directly + * in new functional components. + * + * @deprecated use useReleaseSeries hook instead */ -export default withOrganization(withApi(withTheme(WithRouter))); +export default function ReleaseSeries({children, ...props}: ReleaseSeriesProps) { + const state = useReleaseSeries(props); + return <>{children(state)}; +} diff --git a/static/app/views/insights/common/components/chart.tsx b/static/app/views/insights/common/components/chart.tsx index 23ef214e8da8..6f35920082fd 100644 --- a/static/app/views/insights/common/components/chart.tsx +++ b/static/app/views/insights/common/components/chart.tsx @@ -19,7 +19,7 @@ import ChartZoom, {type ZoomRenderProps} from 'sentry/components/charts/chartZoo import type {FormatterOptions} from 'sentry/components/charts/components/tooltip'; import {getFormatter} from 'sentry/components/charts/components/tooltip'; import {ErrorPanel} from 'sentry/components/charts/errorPanel'; -import ReleaseSeries from 'sentry/components/charts/releaseSeries'; +import {useReleaseSeries} from 'sentry/components/charts/releaseSeries'; import {createLineSeries} from 'sentry/components/charts/series/lineSeries'; import {TransitionChart} from 'sentry/components/charts/transitionChart'; import {TransparentLoadingMask} from 'sentry/components/charts/transparentLoadingMask'; @@ -244,6 +244,17 @@ export function Chart({ const height = renderingContext?.height ?? chartHeight; const isLegendVisible = renderingContext?.isFullscreen ?? showLegend; + const {releaseSeries} = useReleaseSeries({ + start, + end, + queryExtra: undefined, + period, + utc, + projects, + environments, + enabled: renderingContext?.isFullscreen ?? false, + }); + const defaultRef = useRef(null); const chartRef = ref || defaultRef; @@ -472,23 +483,11 @@ export function Chart({ {zoomRenderProps => renderingContext?.isFullscreen ? ( - - {({releaseSeries}) => ( - - )} - + ) : ( ) diff --git a/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx b/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx index 020e4c143db3..35544fc7ab4a 100644 --- a/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx +++ b/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx @@ -13,7 +13,7 @@ import ChartZoom from 'sentry/components/charts/chartZoom'; import {ErrorPanel} from 'sentry/components/charts/errorPanel'; import type {LineChartProps} from 'sentry/components/charts/lineChart'; import {LineChart} from 'sentry/components/charts/lineChart'; -import ReleaseSeries from 'sentry/components/charts/releaseSeries'; +import {useReleaseSeries} from 'sentry/components/charts/releaseSeries'; import {StackedAreaChart} from 'sentry/components/charts/stackedAreaChart'; import {HeaderTitleLegend} from 'sentry/components/charts/styles'; import {TransitionChart} from 'sentry/components/charts/transitionChart'; @@ -67,6 +67,16 @@ function ProjectBaseSessionsChart({ const {projects, environments, datetime} = selection; const {start, end, period, utc} = datetime; + const {releaseSeries} = useReleaseSeries({ + utc, + period, + start, + end, + projects, + environments, + query, + }); + const Request = [DisplayModes.ANR_RATE, DisplayModes.FOREGROUND_ANR_RATE].includes( displayMode ) @@ -95,53 +105,41 @@ function ProjectBaseSessionsChart({ timeseriesData, previousTimeseriesData, additionalSeries, - }) => ( - - {({releaseSeries}) => { - if (errored) { - return ( - - - - ); - } - - return ( - - - - - {title} - {help && } - - - - - ); - }} - - )} + }) => { + if (errored) { + return ( + + + + ); + } + + return ( + + + + + {title} + {help && } + + + + + ); + }} )} From 0a2ae4b14dc596a616d464de8bedaca9d2f2fc95 Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:23:22 +0000 Subject: [PATCH 2/9] :hammer_and_wrench: apply pre-commit fixes --- .../app/components/charts/releaseSeries.tsx | 20 ++++++++++++++----- .../charts/projectBaseSessionsChart.tsx | 4 +--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/static/app/components/charts/releaseSeries.tsx b/static/app/components/charts/releaseSeries.tsx index 8dd2a7206f4e..01193662a389 100644 --- a/static/app/components/charts/releaseSeries.tsx +++ b/static/app/components/charts/releaseSeries.tsx @@ -301,11 +301,11 @@ export function useReleaseSeries({ releases: propReleases, releaseSeries: buildReleaseSeries(propReleases), }); - return undefined; + return; } if (!enabled) { - return undefined; + return; } let cancelled = false; @@ -358,7 +358,17 @@ export function useReleaseSeries({ api.clear(); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [startKey, endKey, period, projectsKey, environmentsKey, query, propReleases, enabled, memoized]); + }, [ + startKey, + endKey, + period, + projectsKey, + environmentsKey, + query, + propReleases, + enabled, + memoized, + ]); // Rebuild series when emphasizeReleases changes without re-fetching. // We use the serialized key (not the array reference) so this effect only @@ -389,7 +399,7 @@ export function useReleaseSeries({ * * @deprecated use useReleaseSeries hook instead */ -export default function ReleaseSeries({children, ...props}: ReleaseSeriesProps) { +export function ReleaseSeries({children, ...props}: ReleaseSeriesProps) { const state = useReleaseSeries(props); - return <>{children(state)}; + return {children(state)}; } diff --git a/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx b/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx index 35544fc7ab4a..d41f448939a8 100644 --- a/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx +++ b/static/app/views/projectDetail/charts/projectBaseSessionsChart.tsx @@ -129,9 +129,7 @@ function ProjectBaseSessionsChart({ reloading={reloading} timeSeries={timeseriesData} previousTimeSeries={ - previousTimeseriesData - ? [previousTimeseriesData] - : undefined + previousTimeseriesData ? [previousTimeseriesData] : undefined } releaseSeries={releaseSeries} displayMode={displayMode} From 37a7da8fd24e0155d689b268ffcbb267bbb1da46 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Wed, 23 Sep 2026 15:02:05 -0700 Subject: [PATCH 3/9] fix(charts): Repair ReleaseSeries imports and tidy the hook useApi and useOrganization are named exports, and eventsChart plus the ReleaseSeries spec still imported the removed default export. Both broke Discover and chart tests at runtime. Also inline the per-call ref aliases in makeOneSeries, drop comments that pointed at the removed class implementation, keep the useReleaseBubbles deprecation, give the exhaustive-deps suppressions a reason, and return children() directly instead of wrapping in a Fragment. --- static/app/components/charts/eventsChart.tsx | 2 +- .../components/charts/releaseSeries.spec.tsx | 20 ++---- .../app/components/charts/releaseSeries.tsx | 71 +++++++------------ 3 files changed, 33 insertions(+), 60 deletions(-) diff --git a/static/app/components/charts/eventsChart.tsx b/static/app/components/charts/eventsChart.tsx index d4d772c51980..1fe1e5b2839c 100644 --- a/static/app/components/charts/eventsChart.tsx +++ b/static/app/components/charts/eventsChart.tsx @@ -21,7 +21,7 @@ import ChartZoom from 'sentry/components/charts/chartZoom'; import {ErrorPanel} from 'sentry/components/charts/errorPanel'; import type {LineChartProps} from 'sentry/components/charts/lineChart'; import {LineChart} from 'sentry/components/charts/lineChart'; -import ReleaseSeries from 'sentry/components/charts/releaseSeries'; +import {ReleaseSeries} from 'sentry/components/charts/releaseSeries'; import {TransitionChart} from 'sentry/components/charts/transitionChart'; import {TransparentLoadingMask} from 'sentry/components/charts/transparentLoadingMask'; import {getInterval, RELEASE_LINES_THRESHOLD} from 'sentry/components/charts/utils'; diff --git a/static/app/components/charts/releaseSeries.spec.tsx b/static/app/components/charts/releaseSeries.spec.tsx index 3e341b17f07f..249cfe40a662 100644 --- a/static/app/components/charts/releaseSeries.spec.tsx +++ b/static/app/components/charts/releaseSeries.spec.tsx @@ -1,13 +1,10 @@ import {Fragment} from 'react'; import {OrganizationFixture} from 'sentry-fixture/organization'; -import {ThemeFixture} from 'sentry-fixture/theme'; import {act, render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; import type {ReleaseSeriesProps} from 'sentry/components/charts/releaseSeries'; -import ReleaseSeries from 'sentry/components/charts/releaseSeries'; - -const theme = ThemeFixture(); +import {ReleaseSeries} from 'sentry/components/charts/releaseSeries'; describe('ReleaseSeries', () => { const renderFunc = jest.fn(() => null); @@ -31,9 +28,7 @@ describe('ReleaseSeries', () => { }); }); - const baseSeriesProps: Omit = { - api: new MockApiClient(), - organization: OrganizationFixture(), + const baseSeriesProps: ReleaseSeriesProps = { period: '14d', start: null, end: null, @@ -42,12 +37,11 @@ describe('ReleaseSeries', () => { query: '', environments: [], children: renderFunc, - theme, }; it('does not fetch releases if releases is truthy', () => { render( - + {renderFunc} ); @@ -57,7 +51,7 @@ describe('ReleaseSeries', () => { it('does not fetch releases if not enabled', () => { render( - + {renderFunc} ); @@ -67,7 +61,7 @@ describe('ReleaseSeries', () => { it('fetches releases if becomes enabled', async () => { const {rerender} = render( - + {renderFunc} ); @@ -75,7 +69,7 @@ describe('ReleaseSeries', () => { expect(releasesMock).not.toHaveBeenCalled(); rerender( - + {renderFunc} ); @@ -85,7 +79,7 @@ describe('ReleaseSeries', () => { expect(releasesMock).toHaveBeenCalledTimes(1); rerender( - + {renderFunc} ); diff --git a/static/app/components/charts/releaseSeries.tsx b/static/app/components/charts/releaseSeries.tsx index 01193662a389..5af5e1d1b2ad 100644 --- a/static/app/components/charts/releaseSeries.tsx +++ b/static/app/components/charts/releaseSeries.tsx @@ -16,10 +16,10 @@ import {escape} from 'sentry/utils'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {getFormat, getFormattedDate, getUtcDateString} from 'sentry/utils/dates'; import {parseLinkHeader} from 'sentry/utils/parseLinkHeader'; -import useApi from 'sentry/utils/useApi'; +import {useApi} from 'sentry/utils/useApi'; import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; -import useOrganization from 'sentry/utils/useOrganization'; +import {useOrganization} from 'sentry/utils/useOrganization'; import {formatVersion} from 'sentry/utils/versions/formatVersion'; import {makeReleasesPathname} from 'sentry/views/explore/releases/utils/pathnames'; @@ -102,7 +102,7 @@ type ReleaseSeriesState = { type UseReleaseSeriesProps = Omit; /** - * Hook that fetches releases and builds ECharts series data for release markers. + * @deprecated use useReleaseBubbles instead */ export function useReleaseSeries({ start, @@ -126,8 +126,7 @@ export function useReleaseSeries({ const location = useLocation(); const navigate = useNavigate(); - // Keep utc in a ref so tooltip formatters always read the latest value - // without needing to be re-created (same pattern as original class used this.props.utc) + // Tooltip formatters live inside echarts and are not re-created when utc changes const utcRef = useRef(utc); useEffect(() => { utcRef.current = utc; @@ -138,11 +137,8 @@ export function useReleaseSeries({ releaseSeries: [], }); - // Serialize non-primitive deps to stable strings so useEffect deps use - // value-equality instead of referential equality. The original class used - // lodash isEqual for these comparisons; callers like Discover rebuild Date - // objects and arrays on every render, so reference equality would re-fetch - // even when nothing logically changed. + // Callers like Discover rebuild Date objects and arrays on every render, so + // effect deps compare serialized values to avoid re-fetching unchanged data. const startKey = start ? getUtcDateString(start) : ''; const endKey = end ? getUtcDateString(end) : ''; const projectsKey = [...projects].join(','); @@ -180,31 +176,23 @@ export function useReleaseSeries({ const releaseSeries: Series[] = []; function makeOneSeries(items: ReleaseMetaBasic[], lineStyle = {}): Series { - const org = organizationRef.current; - const loc = locationRef.current; - const nav = navigateRef.current; - const currentTheme = themeRef.current; - const currentPreserveQueryParams = preserveQueryParamsRef.current; - const currentQueryExtra = queryExtraRef.current; - const currentTooltip = tooltipRef.current; - const currentEnvironments = environmentsRef.current; - const currentStart = startRef.current; - const currentEnd = endRef.current; - const currentPeriod = periodRef.current; - - const extraQuery: Query = {...currentQueryExtra}; - extraQuery.project = loc.query.project; - if (currentPreserveQueryParams) { - extraQuery.environment = [...currentEnvironments]; - extraQuery.start = currentStart ? getUtcDateString(currentStart) : undefined; - extraQuery.end = currentEnd ? getUtcDateString(currentEnd) : undefined; - extraQuery.statsPeriod = currentPeriod || undefined; + const releaseColor = themeRef.current.tokens.dataviz.semantic.release; + + const extraQuery: Query = {...queryExtraRef.current}; + extraQuery.project = locationRef.current.query.project; + if (preserveQueryParamsRef.current) { + extraQuery.environment = [...environmentsRef.current]; + extraQuery.start = startRef.current + ? getUtcDateString(startRef.current) + : undefined; + extraQuery.end = endRef.current ? getUtcDateString(endRef.current) : undefined; + extraQuery.statsPeriod = periodRef.current || undefined; } const markLine = createMarkLine({ animation: false, lineStyle: { - color: currentTheme.tokens.dataviz.semantic.release, + color: releaseColor, opacity: 0.3, type: 'solid', ...lineStyle, @@ -218,9 +206,9 @@ export function useReleaseSeries({ value: formatVersion(release.version, true), onClick: () => { - nav({ + navigateRef.current({ pathname: makeReleasesPathname({ - organization: org, + organization: organizationRef.current, path: `/${encodeURIComponent(release.version)}/`, }), query: extraQuery, @@ -231,15 +219,13 @@ export function useReleaseSeries({ formatter: () => formatVersion(release.version, true), }, })), - tooltip: currentTooltip || { + tooltip: tooltipRef.current || { trigger: 'item', formatter: ({data}: any) => { // Should only happen when navigating pages if (!data) { return ''; } - // Read utc from ref so formatter always gets the latest value - // even though this closure is long-lived inside echarts const time = getFormattedDate( data.value, getFormat({timeZone: true, year: true}), @@ -266,7 +252,7 @@ export function useReleaseSeries({ return { id: 'release-lines', seriesName: 'Releases', - color: currentTheme.tokens.dataviz.semantic.release, + color: releaseColor, data: [], markLine, }; @@ -295,7 +281,6 @@ export function useReleaseSeries({ } useEffect(() => { - // If releases are passed directly via props, skip fetching if (propReleases) { setState({ releases: propReleases, @@ -357,7 +342,7 @@ export function useReleaseSeries({ cancelled = true; api.clear(); }; - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps -- serialized keys stand in for start/end/projects/environments }, [ startKey, endKey, @@ -371,8 +356,6 @@ export function useReleaseSeries({ ]); // Rebuild series when emphasizeReleases changes without re-fetching. - // We use the serialized key (not the array reference) so this effect only - // fires when the actual contents change, not on every render. useEffect(() => { setState(prev => { if (prev.releases === null) { @@ -383,7 +366,7 @@ export function useReleaseSeries({ releaseSeries: buildReleaseSeries(prev.releases), }; }); - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps -- serialized key stands in for emphasizeReleases }, [emphasizeReleasesKey]); if (!enabled) { @@ -394,12 +377,8 @@ export function useReleaseSeries({ } /** - * Render-prop component backed by useReleaseSeries. Prefer the hook directly - * in new functional components. - * * @deprecated use useReleaseSeries hook instead */ export function ReleaseSeries({children, ...props}: ReleaseSeriesProps) { - const state = useReleaseSeries(props); - return {children(state)}; + return children(useReleaseSeries(props)); } From f30bd5d0ea1f1a255c0a2b2c20218e9061dd663c Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 24 Sep 2026 14:47:57 -0700 Subject: [PATCH 4/9] test(charts): Test useReleaseSeries directly The hook is the primary API now, so the spec renders it with renderHookWithProviders instead of going through the render-prop component. The emphasized-release test previously asserted with toHaveBeenCalledWith on the render function, which matched the pre-rerender call and so never checked the post-update series. It now asserts the swapped opacities and that re-emphasizing does not refetch. A new case covers the value-equality deps: rebuilt Date objects and arrays with the same values must not trigger another fetch. --- .../components/charts/releaseSeries.spec.tsx | 320 ++++++------------ 1 file changed, 104 insertions(+), 216 deletions(-) diff --git a/static/app/components/charts/releaseSeries.spec.tsx b/static/app/components/charts/releaseSeries.spec.tsx index 249cfe40a662..2efca476daf7 100644 --- a/static/app/components/charts/releaseSeries.spec.tsx +++ b/static/app/components/charts/releaseSeries.spec.tsx @@ -1,20 +1,17 @@ -import {Fragment} from 'react'; import {OrganizationFixture} from 'sentry-fixture/organization'; -import {act, render, screen, waitFor} from 'sentry-test/reactTestingLibrary'; +import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary'; -import type {ReleaseSeriesProps} from 'sentry/components/charts/releaseSeries'; -import {ReleaseSeries} from 'sentry/components/charts/releaseSeries'; +import {useReleaseSeries} from 'sentry/components/charts/releaseSeries'; -describe('ReleaseSeries', () => { - const renderFunc = jest.fn(() => null); +type Props = Parameters[0]; + +describe('useReleaseSeries', () => { const organization = OrganizationFixture(); let releases: any; let releasesMock: any; beforeEach(() => { - jest.resetAllMocks(); - releases = [ { version: 'sentry-android-shop@1.2.0', @@ -28,7 +25,7 @@ describe('ReleaseSeries', () => { }); }); - const baseSeriesProps: ReleaseSeriesProps = { + const baseProps: Props = { period: '14d', start: null, end: null, @@ -36,79 +33,53 @@ describe('ReleaseSeries', () => { projects: [], query: '', environments: [], - children: renderFunc, }; + function renderReleaseSeries(props: Partial = {}) { + return renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, ...props}, + organization, + }); + } + it('does not fetch releases if releases is truthy', () => { - render( - - {renderFunc} - - ); + renderReleaseSeries({releases: []}); expect(releasesMock).not.toHaveBeenCalled(); }); it('does not fetch releases if not enabled', () => { - render( - - {renderFunc} - - ); + const {result} = renderReleaseSeries({enabled: false}); expect(releasesMock).not.toHaveBeenCalled(); + expect(result.current).toEqual({releases: [], releaseSeries: []}); }); it('fetches releases if becomes enabled', async () => { - const {rerender} = render( - - {renderFunc} - - ); + const {rerender} = renderReleaseSeries({enabled: false}); expect(releasesMock).not.toHaveBeenCalled(); - rerender( - - {renderFunc} - - ); - + rerender({...baseProps, enabled: true}); await act(tick); expect(releasesMock).toHaveBeenCalledTimes(1); - rerender( - - {renderFunc} - - ); - + rerender({...baseProps, enabled: false}); await act(tick); expect(releasesMock).toHaveBeenCalledTimes(1); }); it('fetches releases if no releases passed through props', async () => { - render({renderFunc}); + const {result} = renderReleaseSeries(); expect(releasesMock).toHaveBeenCalled(); - - await waitFor(() => - expect(renderFunc).toHaveBeenCalledWith( - expect.objectContaining({ - releases, - }) - ) - ); + await waitFor(() => expect(result.current.releases).toEqual(releases)); }); it('fetches releases with project conditions', async () => { - render( - - {renderFunc} - - ); + renderReleaseSeries({projects: [1, 2]}); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -121,11 +92,7 @@ describe('ReleaseSeries', () => { }); it('fetches releases with environment conditions', async () => { - render( - - {renderFunc} - - ); + renderReleaseSeries({environments: ['dev', 'test']}); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -138,11 +105,7 @@ describe('ReleaseSeries', () => { }); it('fetches releases with start and end date strings', async () => { - render( - - {renderFunc} - - ); + renderReleaseSeries({start: '2020-01-01', end: '2020-01-31'}); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -160,11 +123,7 @@ describe('ReleaseSeries', () => { it('fetches releases with start and end dates', async () => { const start = new Date(Date.UTC(2020, 0, 1, 12, 13, 14)); const end = new Date(Date.UTC(2020, 0, 31, 14, 15, 16)); - render( - - {renderFunc} - - ); + renderReleaseSeries({start, end}); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -180,11 +139,7 @@ describe('ReleaseSeries', () => { }); it('fetches releases with period', async () => { - render( - - {renderFunc} - - ); + renderReleaseSeries({period: '14d'}); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -197,13 +152,9 @@ describe('ReleaseSeries', () => { }); it('fetches on property updates', async () => { - const wrapper = render( - - {renderFunc} - - ); + const {rerender} = renderReleaseSeries({period: '14d'}); - const cases = [ + const cases: Array> = [ {period: '7d'}, {start: '2020-01-01', end: '2020-01-02'}, {projects: [1]}, @@ -211,11 +162,7 @@ describe('ReleaseSeries', () => { for (const scenario of cases) { releasesMock.mockReset(); - wrapper.rerender( - - {renderFunc} - - ); + rerender({...baseProps, ...scenario}); expect(releasesMock).toHaveBeenCalled(); } @@ -223,174 +170,115 @@ describe('ReleaseSeries', () => { await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); }); - it('doesnt not refetch releases with memoize enabled', async () => { - const originalPeriod = '14d'; - const updatedPeriod = '7d'; - const wrapper = render( - - {renderFunc} - - ); + it('does not refetch when rebuilt dates and arrays are equal', async () => { + const {rerender} = renderReleaseSeries({ + start: new Date(Date.UTC(2020, 0, 1)), + end: new Date(Date.UTC(2020, 0, 2)), + projects: [1], + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); - wrapper.rerender( - - {renderFunc} - - ); + rerender({ + ...baseProps, + start: new Date(Date.UTC(2020, 0, 1)), + end: new Date(Date.UTC(2020, 0, 2)), + projects: [1], + }); + await act(tick); + + expect(releasesMock).toHaveBeenCalledTimes(1); + }); + + it('does not refetch releases with memoize enabled', async () => { + const {rerender} = renderReleaseSeries({period: '14d', memoized: true}); + + await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); + + rerender({...baseProps, period: '7d', memoized: true}); await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(2)); - wrapper.rerender( - - {renderFunc} - - ); + rerender({...baseProps, period: '14d', memoized: true}); await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(2)); }); - it('shares release fetches between components with memoize enabled', async () => { - render( - - - {({releaseSeries}) => { - return releaseSeries.length > 0 ? Series 1 : null; - }} - - - {({releaseSeries}) => { - return releaseSeries.length > 0 ? Series 2 : null; - }} - - - ); + it('shares release fetches between hooks with memoize enabled', async () => { + const first = renderReleaseSeries({period: '42d', memoized: true}); + const second = renderReleaseSeries({period: '42d', memoized: true}); - await screen.findByText('Series 1'); - await screen.findByText('Series 2'); + await waitFor(() => expect(first.result.current.releaseSeries).toHaveLength(1)); + await waitFor(() => expect(second.result.current.releaseSeries).toHaveLength(1)); - await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); + expect(releasesMock).toHaveBeenCalledTimes(1); }); it('generates an eCharts `markLine` series from releases', async () => { - render({renderFunc}); + const {result} = renderReleaseSeries(); await waitFor(() => - expect(renderFunc).toHaveBeenCalledWith( + expect(result.current.releaseSeries).toEqual([ expect.objectContaining({ - releaseSeries: [ - expect.objectContaining({ - // we don't care about the other properties for now - markLine: expect.objectContaining({ - data: [ - expect.objectContaining({ - name: '1.2.0, sentry-android-shop', - value: '1.2.0, sentry-android-shop', - xAxis: 1584921600000, - }), - ], + markLine: expect.objectContaining({ + data: [ + expect.objectContaining({ + name: '1.2.0, sentry-android-shop', + value: '1.2.0, sentry-android-shop', + xAxis: 1584921600000, }), - }), - ], - }) - ) + ], + }), + }), + ]) ); }); - it('allows updating the emphasized release', async () => { + it('allows updating the emphasized release without refetching', async () => { releases.push({ version: 'sentry-android-shop@1.2.1', date: '2020-03-24T00:00:00Z', }); - const wrapper = render( - - {renderFunc} - - ); + const {result, rerender} = renderReleaseSeries({ + emphasizeReleases: ['sentry-android-shop@1.2.0'], + }); + // Unemphasized releases render at opacity 0.3, emphasized at 0.8 await waitFor(() => - expect(renderFunc).toHaveBeenCalledWith( + expect(result.current.releaseSeries).toEqual([ expect.objectContaining({ - releaseSeries: [ - expect.objectContaining({ - // we don't care about the other properties for now - markLine: expect.objectContaining({ - // the unemphasized releases have opacity 0.3 - lineStyle: expect.objectContaining({opacity: 0.3}), - data: [ - expect.objectContaining({ - name: '1.2.1, sentry-android-shop', - value: '1.2.1, sentry-android-shop', - xAxis: 1585008000000, - }), - ], - }), - }), - expect.objectContaining({ - // we don't care about the other properties for now - markLine: expect.objectContaining({ - // the emphasized releases have opacity 0.8 - lineStyle: expect.objectContaining({opacity: 0.8}), - data: [ - expect.objectContaining({ - name: '1.2.0, sentry-android-shop', - value: '1.2.0, sentry-android-shop', - xAxis: 1584921600000, - }), - ], - }), - }), - ], - }) - ) + markLine: expect.objectContaining({ + lineStyle: expect.objectContaining({opacity: 0.3}), + data: [expect.objectContaining({name: '1.2.1, sentry-android-shop'})], + }), + }), + expect.objectContaining({ + markLine: expect.objectContaining({ + lineStyle: expect.objectContaining({opacity: 0.8}), + data: [expect.objectContaining({name: '1.2.0, sentry-android-shop'})], + }), + }), + ]) ); - wrapper.rerender( - - {renderFunc} - - ); + rerender({...baseProps, emphasizeReleases: ['sentry-android-shop@1.2.1']}); - expect(renderFunc).toHaveBeenCalledWith( - expect.objectContaining({ - releaseSeries: [ - expect.objectContaining({ - // we don't care about the other properties for now - markLine: expect.objectContaining({ - // the unemphasized releases have opacity 0.3 - lineStyle: expect.objectContaining({opacity: 0.3}), - data: [ - expect.objectContaining({ - name: '1.2.1, sentry-android-shop', - value: '1.2.1, sentry-android-shop', - xAxis: 1585008000000, - }), - ], - }), + await waitFor(() => + expect(result.current.releaseSeries).toEqual([ + expect.objectContaining({ + markLine: expect.objectContaining({ + lineStyle: expect.objectContaining({opacity: 0.3}), + data: [expect.objectContaining({name: '1.2.0, sentry-android-shop'})], }), - expect.objectContaining({ - // we don't care about the other properties for now - markLine: expect.objectContaining({ - // the emphasized releases have opacity 0.8 - lineStyle: expect.objectContaining({opacity: 0.8}), - data: [ - expect.objectContaining({ - name: '1.2.0, sentry-android-shop', - value: '1.2.0, sentry-android-shop', - xAxis: 1584921600000, - }), - ], - }), + }), + expect.objectContaining({ + markLine: expect.objectContaining({ + lineStyle: expect.objectContaining({opacity: 0.8}), + data: [expect.objectContaining({name: '1.2.1, sentry-android-shop'})], }), - ], - }) + }), + ]) ); + expect(releasesMock).toHaveBeenCalledTimes(1); }); }); From 9046d6a3a3c3af5900698fe67df79e9373735de1 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 24 Sep 2026 14:58:33 -0700 Subject: [PATCH 5/9] fix(charts): Derive release series during render eslint-plugin-react-you-might-not-need-an-effect 1.0 flags the effect that rebuilt the series from state when emphasizeReleases changed (no-derived-state). Only the fetched releases live in state now; the series is memoized from releases, emphasizeReleases, theme, tooltip and utc, and releases passed through props are used directly. The markLine click handler reads navigation params from a ref at click time, so callers that pass freshly-allocated arrays or dates don't rebuild the series on every render. The tooltip formatter closes over utc, which is a memo dependency, so the utc ref is no longer needed. --- .../app/components/charts/releaseSeries.tsx | 324 +++++++++--------- 1 file changed, 153 insertions(+), 171 deletions(-) diff --git a/static/app/components/charts/releaseSeries.tsx b/static/app/components/charts/releaseSeries.tsx index 5af5e1d1b2ad..dbb6c676a689 100644 --- a/static/app/components/charts/releaseSeries.tsx +++ b/static/app/components/charts/releaseSeries.tsx @@ -1,4 +1,4 @@ -import {useEffect, useRef, useState} from 'react'; +import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {useTheme} from '@emotion/react'; import type {Query} from 'history'; import memoize from 'lodash/memoize'; @@ -101,6 +101,97 @@ type ReleaseSeriesState = { type UseReleaseSeriesProps = Omit; +function buildReleaseSeries({ + releases, + emphasizeReleases, + color, + tooltip, + utc, + onReleaseClick, +}: { + color: string; + onReleaseClick: (version: string) => void; + releases: ReleaseMetaBasic[]; + emphasizeReleases?: string[]; + tooltip?: ReleaseSeriesProps['tooltip']; + utc?: boolean | null; +}): Series[] { + function makeOneSeries(items: ReleaseMetaBasic[], lineStyle = {}): Series { + const markLine = createMarkLine({ + animation: false, + lineStyle: { + color, + opacity: 0.3, + type: 'solid', + ...lineStyle, + }, + label: { + show: false, + }, + data: items.map(release => ({ + xAxis: +new Date(release.date), + name: formatVersion(release.version, true), + value: formatVersion(release.version, true), + onClick: () => onReleaseClick(release.version), + label: { + formatter: () => formatVersion(release.version, true), + }, + })), + tooltip: tooltip || { + trigger: 'item', + formatter: ({data}: any) => { + // Should only happen when navigating pages + if (!data) { + return ''; + } + const time = getFormattedDate( + data.value, + getFormat({timeZone: true, year: true}), + {local: !utc} + ); + const version = escape(formatVersion(data.name, true)); + return [ + '
', + `
${t( + 'Release' + )} ${version}
`, + '
', + '
', + time, + '
', + '
', + ].join(''); + }, + }, + }); + + return { + id: 'release-lines', + seriesName: 'Releases', + color, + data: [], + markLine, + }; + } + + if (!emphasizeReleases?.length) { + return [makeOneSeries(releases)]; + } + + const [unemphasizedReleases, emphasizedReleases] = partition( + releases, + release => !emphasizeReleases.includes(release.version) + ); + const releaseSeries: Series[] = []; + if (unemphasizedReleases.length) { + releaseSeries.push(makeOneSeries(unemphasizedReleases, {type: 'dotted'})); + } + if (emphasizedReleases.length) { + releaseSeries.push(makeOneSeries(emphasizedReleases, {opacity: 0.8})); + } + return releaseSeries; +} + /** * @deprecated use useReleaseBubbles instead */ @@ -126,16 +217,8 @@ export function useReleaseSeries({ const location = useLocation(); const navigate = useNavigate(); - // Tooltip formatters live inside echarts and are not re-created when utc changes - const utcRef = useRef(utc); - useEffect(() => { - utcRef.current = utc; - }, [utc]); - - const [state, setState] = useState({ - releases: null, - releaseSeries: [], - }); + const [fetchedReleases, setFetchedReleases] = useState(null); + const releases = propReleases ?? fetchedReleases; // Callers like Discover rebuild Date objects and arrays on every render, so // effect deps compare serialized values to avoid re-fetching unchanged data. @@ -143,153 +226,69 @@ export function useReleaseSeries({ const endKey = end ? getUtcDateString(end) : ''; const projectsKey = [...projects].join(','); const environmentsKey = [...environments].join(','); - const emphasizeReleasesKey = emphasizeReleases?.join(',') ?? ''; - - // Stable refs for values used inside closures passed to echarts - const organizationRef = useRef(organization); - const locationRef = useRef(location); - const navigateRef = useRef(navigate); - const themeRef = useRef(theme); - const preserveQueryParamsRef = useRef(preserveQueryParams); - const queryExtraRef = useRef(queryExtra); - const tooltipRef = useRef(tooltip); - const environmentsRef = useRef(environments); - const startRef = useRef(start); - const endRef = useRef(end); - const periodRef = useRef(period); + // Read at click time so the memoized series doesn't rebuild whenever a + // caller passes freshly-allocated navigation params. + const clickContextRef = useRef({ + environments, + end, + location, + navigate, + organization, + period, + preserveQueryParams, + queryExtra, + start, + }); useEffect(() => { - organizationRef.current = organization; - locationRef.current = location; - navigateRef.current = navigate; - themeRef.current = theme; - preserveQueryParamsRef.current = preserveQueryParams; - queryExtraRef.current = queryExtra; - tooltipRef.current = tooltip; - environmentsRef.current = environments; - startRef.current = start; - endRef.current = end; - periodRef.current = period; + clickContextRef.current = { + environments, + end, + location, + navigate, + organization, + period, + preserveQueryParams, + queryExtra, + start, + }; }); - function buildReleaseSeries(releases: ReleaseMetaBasic[]): Series[] { - const releaseSeries: Series[] = []; - - function makeOneSeries(items: ReleaseMetaBasic[], lineStyle = {}): Series { - const releaseColor = themeRef.current.tokens.dataviz.semantic.release; - - const extraQuery: Query = {...queryExtraRef.current}; - extraQuery.project = locationRef.current.query.project; - if (preserveQueryParamsRef.current) { - extraQuery.environment = [...environmentsRef.current]; - extraQuery.start = startRef.current - ? getUtcDateString(startRef.current) - : undefined; - extraQuery.end = endRef.current ? getUtcDateString(endRef.current) : undefined; - extraQuery.statsPeriod = periodRef.current || undefined; - } - - const markLine = createMarkLine({ - animation: false, - lineStyle: { - color: releaseColor, - opacity: 0.3, - type: 'solid', - ...lineStyle, - }, - label: { - show: false, - }, - data: items.map((release: ReleaseMetaBasic) => ({ - xAxis: +new Date(release.date), - name: formatVersion(release.version, true), - value: formatVersion(release.version, true), - - onClick: () => { - navigateRef.current({ - pathname: makeReleasesPathname({ - organization: organizationRef.current, - path: `/${encodeURIComponent(release.version)}/`, - }), - query: extraQuery, - }); - }, - - label: { - formatter: () => formatVersion(release.version, true), - }, - })), - tooltip: tooltipRef.current || { - trigger: 'item', - formatter: ({data}: any) => { - // Should only happen when navigating pages - if (!data) { - return ''; - } - const time = getFormattedDate( - data.value, - getFormat({timeZone: true, year: true}), - { - local: !utcRef.current, - } - ); - const version = escape(formatVersion(data.name, true)); - return [ - '
', - `
${t( - 'Release' - )} ${version}
`, - '
', - '
', - time, - '
', - '
', - ].join(''); - }, - }, - }); - - return { - id: 'release-lines', - seriesName: 'Releases', - color: releaseColor, - data: [], - markLine, - }; + const handleReleaseClick = useCallback((version: string) => { + const ctx = clickContextRef.current; + const extraQuery: Query = {...ctx.queryExtra, project: ctx.location.query.project}; + if (ctx.preserveQueryParams) { + extraQuery.environment = [...ctx.environments]; + extraQuery.start = ctx.start ? getUtcDateString(ctx.start) : undefined; + extraQuery.end = ctx.end ? getUtcDateString(ctx.end) : undefined; + extraQuery.statsPeriod = ctx.period || undefined; } - - if (emphasizeReleases?.length) { - const [unemphasizedReleases, emphasizedReleases] = partition( - releases, - release => !emphasizeReleases!.includes(release.version) - ); - if (unemphasizedReleases.length) { - releaseSeries.push(makeOneSeries(unemphasizedReleases, {type: 'dotted'})); - } - if (emphasizedReleases.length) { - releaseSeries.push( - makeOneSeries(emphasizedReleases, { - opacity: 0.8, + ctx.navigate({ + pathname: makeReleasesPathname({ + organization: ctx.organization, + path: `/${encodeURIComponent(version)}/`, + }), + query: extraQuery, + }); + }, []); + + const releaseSeries = useMemo( + () => + releases + ? buildReleaseSeries({ + releases, + emphasizeReleases, + color: theme.tokens.dataviz.semantic.release, + tooltip, + utc, + onReleaseClick: handleReleaseClick, }) - ); - } - } else { - releaseSeries.push(makeOneSeries(releases)); - } - - return releaseSeries; - } + : [], + [releases, emphasizeReleases, theme, tooltip, utc, handleReleaseClick] + ); useEffect(() => { - if (propReleases) { - setState({ - releases: propReleases, - releaseSeries: buildReleaseSeries(propReleases), - }); - return; - } - - if (!enabled) { + if (propReleases || !enabled) { return; } @@ -306,19 +305,16 @@ export function useReleaseSeries({ }; let hasMore = true; - const releases: ReleaseMetaBasic[] = []; + const allReleases: ReleaseMetaBasic[] = []; while (hasMore) { try { const getReleases = memoized ? getOrganizationReleasesMemoized : getOrganizationReleases; const [newReleases, , resp] = await getReleases(api, organization, conditions); - releases.push(...newReleases); + allReleases.push(...newReleases); if (!cancelled) { - setState({ - releases, - releaseSeries: buildReleaseSeries(releases), - }); + setFetchedReleases([...allReleases]); } const pageLinks = resp?.getResponseHeader('Link'); @@ -355,25 +351,11 @@ export function useReleaseSeries({ memoized, ]); - // Rebuild series when emphasizeReleases changes without re-fetching. - useEffect(() => { - setState(prev => { - if (prev.releases === null) { - return prev; - } - return { - ...prev, - releaseSeries: buildReleaseSeries(prev.releases), - }; - }); - // eslint-disable-next-line react-hooks/exhaustive-deps -- serialized key stands in for emphasizeReleases - }, [emphasizeReleasesKey]); - if (!enabled) { return {releases: [], releaseSeries: []}; } - return state; + return {releases, releaseSeries}; } /** From f42355fa7d92b3befbcc96251f76e4c828df470d Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 24 Sep 2026 15:12:18 -0700 Subject: [PATCH 6/9] ref(charts): Remove the ReleaseSeries render-prop component EventsChart was the last caller. Every prop it passed came from its own props rather than from the ChartZoom/EventsRequest render args, so it can call useReleaseSeries at the top level with enabled: !disableReleases. The hook returns an empty series when disabled, which matches what the chart received before. With the component gone, UseReleaseSeriesProps is the only props type and ChartDataProps no longer carries releaseSeries. --- static/app/components/charts/eventsChart.tsx | 40 ++++++++----------- .../app/components/charts/releaseSeries.tsx | 14 +------ 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/static/app/components/charts/eventsChart.tsx b/static/app/components/charts/eventsChart.tsx index 1fe1e5b2839c..870ca1eb6b7e 100644 --- a/static/app/components/charts/eventsChart.tsx +++ b/static/app/components/charts/eventsChart.tsx @@ -21,7 +21,7 @@ import ChartZoom from 'sentry/components/charts/chartZoom'; import {ErrorPanel} from 'sentry/components/charts/errorPanel'; import type {LineChartProps} from 'sentry/components/charts/lineChart'; import {LineChart} from 'sentry/components/charts/lineChart'; -import {ReleaseSeries} from 'sentry/components/charts/releaseSeries'; +import {useReleaseSeries} from 'sentry/components/charts/releaseSeries'; import {TransitionChart} from 'sentry/components/charts/transitionChart'; import {TransparentLoadingMask} from 'sentry/components/charts/transparentLoadingMask'; import {getInterval, RELEASE_LINES_THRESHOLD} from 'sentry/components/charts/utils'; @@ -435,7 +435,6 @@ type ChartDataProps = { reloading: boolean; zoomRenderProps: ZoomRenderProps; previousTimeseriesData?: Series[] | null; - releaseSeries?: Series[]; results?: Series[]; tableData?: TableDataWithTitle[]; timeframe?: {end: number; start: number}; @@ -511,9 +510,21 @@ export function EventsChart(props: EventsChartProps) { const intervalVal = showDaily ? '1d' : interval || getInterval(props, 'high'); - let chartImplementation = ({ + const {releaseSeries} = useReleaseSeries({ + utc, + period, + start, + end, + projects, + environments, + emphasizeReleases, + preserveQueryParams: preserveReleaseQueryParams, + queryExtra: releaseQueryExtra, + enabled: !disableReleases, + }); + + const chartImplementation = ({ zoomRenderProps, - releaseSeries, errored, loading, reloading, @@ -550,7 +561,7 @@ export function EventsChart(props: EventsChartProps) { reloading={reloading || !!reloadingAdditionalSeries} showLegend={showLegend} minutesThresholdToDisplaySeconds={minutesThresholdToDisplaySeconds} - releaseSeries={releaseSeries || []} + releaseSeries={releaseSeries} timeseriesData={seriesData ?? []} previousTimeseriesData={previousTimeseriesData} currentSeriesNames={currentSeriesNames} @@ -577,25 +588,6 @@ export function EventsChart(props: EventsChartProps) { ); }; - if (!disableReleases) { - const previousChart = chartImplementation; - chartImplementation = chartProps => ( - - {({releaseSeries}) => previousChart({...chartProps, releaseSeries})} - - ); - } - return ( React.ReactNode; +interface UseReleaseSeriesProps { end: DateString; environments: readonly string[]; projects: readonly number[]; @@ -99,8 +98,6 @@ type ReleaseSeriesState = { releases: ReleaseMetaBasic[] | null; }; -type UseReleaseSeriesProps = Omit; - function buildReleaseSeries({ releases, emphasizeReleases, @@ -113,7 +110,7 @@ function buildReleaseSeries({ onReleaseClick: (version: string) => void; releases: ReleaseMetaBasic[]; emphasizeReleases?: string[]; - tooltip?: ReleaseSeriesProps['tooltip']; + tooltip?: UseReleaseSeriesProps['tooltip']; utc?: boolean | null; }): Series[] { function makeOneSeries(items: ReleaseMetaBasic[], lineStyle = {}): Series { @@ -357,10 +354,3 @@ export function useReleaseSeries({ return {releases, releaseSeries}; } - -/** - * @deprecated use useReleaseSeries hook instead - */ -export function ReleaseSeries({children, ...props}: ReleaseSeriesProps) { - return children(useReleaseSeries(props)); -} From 42da16933cc0d24f86a045934259160de25b6634 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 24 Sep 2026 15:37:32 -0700 Subject: [PATCH 7/9] test(charts): Call renderHookWithProviders directly in useReleaseSeries spec Each test now shows exactly what it renders instead of routing through a local wrapper. --- .../components/charts/releaseSeries.spec.tsx | 92 +++++++++++++------ 1 file changed, 66 insertions(+), 26 deletions(-) diff --git a/static/app/components/charts/releaseSeries.spec.tsx b/static/app/components/charts/releaseSeries.spec.tsx index 2efca476daf7..29db8321a691 100644 --- a/static/app/components/charts/releaseSeries.spec.tsx +++ b/static/app/components/charts/releaseSeries.spec.tsx @@ -35,28 +35,30 @@ describe('useReleaseSeries', () => { environments: [], }; - function renderReleaseSeries(props: Partial = {}) { - return renderHookWithProviders(useReleaseSeries, { - initialProps: {...baseProps, ...props}, + it('does not fetch releases if releases is truthy', () => { + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, releases: []}, organization, }); - } - - it('does not fetch releases if releases is truthy', () => { - renderReleaseSeries({releases: []}); expect(releasesMock).not.toHaveBeenCalled(); }); it('does not fetch releases if not enabled', () => { - const {result} = renderReleaseSeries({enabled: false}); + const {result} = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, enabled: false}, + organization, + }); expect(releasesMock).not.toHaveBeenCalled(); expect(result.current).toEqual({releases: [], releaseSeries: []}); }); it('fetches releases if becomes enabled', async () => { - const {rerender} = renderReleaseSeries({enabled: false}); + const {rerender} = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, enabled: false}, + organization, + }); expect(releasesMock).not.toHaveBeenCalled(); @@ -72,14 +74,20 @@ describe('useReleaseSeries', () => { }); it('fetches releases if no releases passed through props', async () => { - const {result} = renderReleaseSeries(); + const {result} = renderHookWithProviders(useReleaseSeries, { + initialProps: baseProps, + organization, + }); expect(releasesMock).toHaveBeenCalled(); await waitFor(() => expect(result.current.releases).toEqual(releases)); }); it('fetches releases with project conditions', async () => { - renderReleaseSeries({projects: [1, 2]}); + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, projects: [1, 2]}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -92,7 +100,10 @@ describe('useReleaseSeries', () => { }); it('fetches releases with environment conditions', async () => { - renderReleaseSeries({environments: ['dev', 'test']}); + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, environments: ['dev', 'test']}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -105,7 +116,10 @@ describe('useReleaseSeries', () => { }); it('fetches releases with start and end date strings', async () => { - renderReleaseSeries({start: '2020-01-01', end: '2020-01-31'}); + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, start: '2020-01-01', end: '2020-01-31'}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -123,7 +137,10 @@ describe('useReleaseSeries', () => { it('fetches releases with start and end dates', async () => { const start = new Date(Date.UTC(2020, 0, 1, 12, 13, 14)); const end = new Date(Date.UTC(2020, 0, 31, 14, 15, 16)); - renderReleaseSeries({start, end}); + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, start, end}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -139,7 +156,10 @@ describe('useReleaseSeries', () => { }); it('fetches releases with period', async () => { - renderReleaseSeries({period: '14d'}); + renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, period: '14d'}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledWith( @@ -152,7 +172,10 @@ describe('useReleaseSeries', () => { }); it('fetches on property updates', async () => { - const {rerender} = renderReleaseSeries({period: '14d'}); + const {rerender} = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, period: '14d'}, + organization, + }); const cases: Array> = [ {period: '7d'}, @@ -171,10 +194,14 @@ describe('useReleaseSeries', () => { }); it('does not refetch when rebuilt dates and arrays are equal', async () => { - const {rerender} = renderReleaseSeries({ - start: new Date(Date.UTC(2020, 0, 1)), - end: new Date(Date.UTC(2020, 0, 2)), - projects: [1], + const {rerender} = renderHookWithProviders(useReleaseSeries, { + initialProps: { + ...baseProps, + start: new Date(Date.UTC(2020, 0, 1)), + end: new Date(Date.UTC(2020, 0, 2)), + projects: [1], + }, + organization, }); await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); @@ -191,7 +218,10 @@ describe('useReleaseSeries', () => { }); it('does not refetch releases with memoize enabled', async () => { - const {rerender} = renderReleaseSeries({period: '14d', memoized: true}); + const {rerender} = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, period: '14d', memoized: true}, + organization, + }); await waitFor(() => expect(releasesMock).toHaveBeenCalledTimes(1)); @@ -205,8 +235,14 @@ describe('useReleaseSeries', () => { }); it('shares release fetches between hooks with memoize enabled', async () => { - const first = renderReleaseSeries({period: '42d', memoized: true}); - const second = renderReleaseSeries({period: '42d', memoized: true}); + const first = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, period: '42d', memoized: true}, + organization, + }); + const second = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, period: '42d', memoized: true}, + organization, + }); await waitFor(() => expect(first.result.current.releaseSeries).toHaveLength(1)); await waitFor(() => expect(second.result.current.releaseSeries).toHaveLength(1)); @@ -215,7 +251,10 @@ describe('useReleaseSeries', () => { }); it('generates an eCharts `markLine` series from releases', async () => { - const {result} = renderReleaseSeries(); + const {result} = renderHookWithProviders(useReleaseSeries, { + initialProps: baseProps, + organization, + }); await waitFor(() => expect(result.current.releaseSeries).toEqual([ @@ -239,8 +278,9 @@ describe('useReleaseSeries', () => { version: 'sentry-android-shop@1.2.1', date: '2020-03-24T00:00:00Z', }); - const {result, rerender} = renderReleaseSeries({ - emphasizeReleases: ['sentry-android-shop@1.2.0'], + const {result, rerender} = renderHookWithProviders(useReleaseSeries, { + initialProps: {...baseProps, emphasizeReleases: ['sentry-android-shop@1.2.0']}, + organization, }); // Unemphasized releases render at opacity 0.3, emphasized at 0.8 From b114451820a97b9dd7edb557d1f797770ac22630 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 24 Sep 2026 15:41:16 -0700 Subject: [PATCH 8/9] test(charts): Type the property-update rerender through baseProps renderHookWithProviders infers its props type from initialProps, so {...baseProps, period: '14d'} narrowed period to a required string and rerender() then rejected Partial spreads. baseProps already sets that period and carries the full Props type. --- static/app/components/charts/releaseSeries.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/charts/releaseSeries.spec.tsx b/static/app/components/charts/releaseSeries.spec.tsx index 29db8321a691..2985503bd727 100644 --- a/static/app/components/charts/releaseSeries.spec.tsx +++ b/static/app/components/charts/releaseSeries.spec.tsx @@ -173,7 +173,7 @@ describe('useReleaseSeries', () => { it('fetches on property updates', async () => { const {rerender} = renderHookWithProviders(useReleaseSeries, { - initialProps: {...baseProps, period: '14d'}, + initialProps: baseProps, organization, }); From a187f80c219bdcbe2b43ba03a32f0eceeaf5f64e Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Fri, 25 Sep 2026 16:59:07 -0700 Subject: [PATCH 9/9] test(charts): Restore value and xAxis checks in emphasized-release test The hook migration narrowed each release data matcher to name only. Match name, value and xAxis again, as the original spec did. --- .../components/charts/releaseSeries.spec.tsx | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/static/app/components/charts/releaseSeries.spec.tsx b/static/app/components/charts/releaseSeries.spec.tsx index 2985503bd727..38e4788ad29e 100644 --- a/static/app/components/charts/releaseSeries.spec.tsx +++ b/static/app/components/charts/releaseSeries.spec.tsx @@ -283,19 +283,30 @@ describe('useReleaseSeries', () => { organization, }); + const release120 = expect.objectContaining({ + name: '1.2.0, sentry-android-shop', + value: '1.2.0, sentry-android-shop', + xAxis: 1584921600000, + }); + const release121 = expect.objectContaining({ + name: '1.2.1, sentry-android-shop', + value: '1.2.1, sentry-android-shop', + xAxis: 1585008000000, + }); + // Unemphasized releases render at opacity 0.3, emphasized at 0.8 await waitFor(() => expect(result.current.releaseSeries).toEqual([ expect.objectContaining({ markLine: expect.objectContaining({ lineStyle: expect.objectContaining({opacity: 0.3}), - data: [expect.objectContaining({name: '1.2.1, sentry-android-shop'})], + data: [release121], }), }), expect.objectContaining({ markLine: expect.objectContaining({ lineStyle: expect.objectContaining({opacity: 0.8}), - data: [expect.objectContaining({name: '1.2.0, sentry-android-shop'})], + data: [release120], }), }), ]) @@ -308,13 +319,13 @@ describe('useReleaseSeries', () => { expect.objectContaining({ markLine: expect.objectContaining({ lineStyle: expect.objectContaining({opacity: 0.3}), - data: [expect.objectContaining({name: '1.2.0, sentry-android-shop'})], + data: [release120], }), }), expect.objectContaining({ markLine: expect.objectContaining({ lineStyle: expect.objectContaining({opacity: 0.8}), - data: [expect.objectContaining({name: '1.2.1, sentry-android-shop'})], + data: [release121], }), }), ])