From 9b9178c215048b193bd74ca0bcfd3c5074f57aa4 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 11:46:32 +0200 Subject: [PATCH 1/7] Coordinated TTID/TTFD --- .../js/tracing/timeToDisplayCoordinator.ts | 139 +++++++++++ .../core/src/js/tracing/timetodisplay.tsx | 96 +++++++- .../tracing/timeToDisplayCoordinator.test.ts | 127 ++++++++++ .../timetodisplay.multiinstance.test.tsx | 225 ++++++++++++++++++ 4 files changed, 580 insertions(+), 7 deletions(-) create mode 100644 packages/core/src/js/tracing/timeToDisplayCoordinator.ts create mode 100644 packages/core/test/tracing/timeToDisplayCoordinator.test.ts create mode 100644 packages/core/test/tracing/timetodisplay.multiinstance.test.tsx diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts new file mode 100644 index 0000000000..179bf1ff17 --- /dev/null +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -0,0 +1,139 @@ +/** + * Coordinator for multi-instance `` / `` + * components on a single screen (active span). + */ + +type Checkpoint = { ready: boolean }; +type Listener = () => void; + +interface SpanRegistry { + checkpoints: Map; + listeners: Set; +} + +const TTID = 'ttid'; +const TTFD = 'ttfd'; + +export type DisplayKind = typeof TTID | typeof TTFD; + +const registries: Record> = { + ttid: new Map(), + ttfd: new Map(), +}; + +function getOrCreate(kind: DisplayKind, parentSpanId: string): SpanRegistry { + const map = registries[kind]; + let entry = map.get(parentSpanId); + if (!entry) { + entry = { + checkpoints: new Map(), + listeners: new Set() + }; + map.set(parentSpanId, entry); + } + return entry; +} + +function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { + if (entry.checkpoints.size === 0 && entry.listeners.size === 0) { + registries[kind].delete(parentSpanId); + } +} + +/** + * Register a checkpoint under (kind, parentSpanId). Returns an unregister fn. + */ +export function registerCheckpoint( + kind: DisplayKind, + parentSpanId: string, + checkpointId: string, + ready: boolean, +): () => void { + const entry = getOrCreate(kind, parentSpanId); + entry.checkpoints.set(checkpointId, { ready }); + notify(entry); + + return () => { + const e = registries[kind].get(parentSpanId); + if (!e) { + return; + } + if (e.checkpoints.delete(checkpointId)) { + notify(e); + } + performCleanup(kind, parentSpanId, e); + }; +} + +/** + * Update an existing checkpoint's ready state. + */ +export function updateCheckpoint( + kind: DisplayKind, + parentSpanId: string, + checkpointId: string, + ready: boolean, +): void { + const entry = registries[kind].get(parentSpanId); + const cp = entry?.checkpoints.get(checkpointId); + if (!entry || !cp || cp.ready === ready) { + return; + } + cp.ready = ready; + notify(entry); +} + +/** + * True if at least one checkpoint is registered AND all checkpoints are ready. + */ +export function isAllReady(kind: DisplayKind, parentSpanId: string): boolean { + const entry = registries[kind].get(parentSpanId); + if (!entry || entry.checkpoints.size === 0) { + return false; + } + for (const cp of entry.checkpoints.values()) { + if (!cp.ready) { + return false; + } + } + return true; +} + +/** + * Returns true if there is at least one registered checkpoint on this span. + */ +export function hasAnyCheckpoints(kind: DisplayKind, parentSpanId: string): boolean { + const entry = registries[kind].get(parentSpanId); + return !!entry && entry.checkpoints.size > 0; +} + +/** + * Subscribe to any checkpoint state change for a given span. The listener is + * called synchronously after each register/update/unregister event. + */ +export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Listener): () => void { + const entry = getOrCreate(kind, parentSpanId); + entry.listeners.add(listener); + return () => { + const e = registries[kind].get(parentSpanId); + if (!e) { + return; + } + e.listeners.delete(listener); + performCleanup(kind, parentSpanId, e); + }; +} + +function notify(entry: SpanRegistry): void { + for (const listener of entry.listeners) { + listener(); + } +} + +/** + * Test-only. Clears all coordinator state. + */ +export function _resetTimeToDisplayCoordinator(): void { + registries.ttid.clear(); + registries.ttfd.clear(); +} diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index 7bb74445b5..f6bbb8119c 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -13,12 +13,14 @@ import { startInactiveSpan, } from '@sentry/core'; import * as React from 'react'; -import { useState } from 'react'; +import { useEffect, useId, useReducer, useState } from 'react'; import type { NativeFramesResponse } from '../NativeRNSentry'; import { NATIVE } from '../wrapper'; import { SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY, SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY } from './origin'; +import type { DisplayKind } from './timeToDisplayCoordinator'; +import { isAllReady, registerCheckpoint, subscribe, updateCheckpoint } from './timeToDisplayCoordinator'; import { getRNSentryOnDrawReporter } from './timetodisplaynative'; import { setSpanDurationAsMeasurement, setSpanDurationAsMeasurementOnSpan } from './utils'; @@ -59,15 +61,31 @@ const spanFrameDataMap = new Map(); export type TimeToDisplayProps = { children?: React.ReactNode; + /** + * @deprecated Use `ready` instead. `record` and `ready` are equivalent; + * `record` will be removed in the next major version. + */ record?: boolean; + /** + * Marks this checkpoint as ready. The display is recorded only when every + * `` / `` mounted under the + * currently active span reports `ready === true`. + * + * + * + */ + ready?: boolean; }; /** * Component to measure time to initial display. * - * The initial display is recorded when the component prop `record` is true. + * Single instance: + * * - * + * Multiple instances coordinating on one screen: + * + * */ export function TimeToInitialDisplay(props: TimeToDisplayProps): React.ReactElement { const activeSpan = getActiveSpan(); @@ -76,8 +94,10 @@ export function TimeToInitialDisplay(props: TimeToDisplayProps): React.ReactElem } const parentSpanId = activeSpan && spanToJSON(activeSpan).span_id; + const initialDisplay = useCoordinatedDisplay('ttid', parentSpanId, props); + return ( - + {props.children} ); @@ -86,20 +106,82 @@ export function TimeToInitialDisplay(props: TimeToDisplayProps): React.ReactElem /** * Component to measure time to full display. * - * The initial display is recorded when the component prop `record` is true. + * Single instance: + * * - * + * Multiple instances coordinating on one screen: + * + * */ export function TimeToFullDisplay(props: TimeToDisplayProps): React.ReactElement { const activeSpan = getActiveSpan(); const parentSpanId = activeSpan && spanToJSON(activeSpan).span_id; + const fullDisplay = useCoordinatedDisplay('ttfd', parentSpanId, props); + return ( - + {props.children} ); } +/** + * Every `` / `` instance registers as + * a checkpoint under the active span. The aggregate is ready if every + * checkpoint reports ready. + */ +function useCoordinatedDisplay( + kind: DisplayKind, + parentSpanId: string | undefined, + props: TimeToDisplayProps, +): boolean { + const checkpointId = useId(); + const [, force] = useReducer((x: number) => x + 1, 0); + + // `ready` takes precedence when both are provided. + const localReady = props.ready !== undefined ? !!props.ready : !!props.record; + + if (__DEV__) { + if (props.ready !== undefined && props.record !== undefined) { + debug.warn('[TimeToDisplay] Both `ready` and `record` were provided — ignoring `record`.'); + } + if (props.record !== undefined) { + debug.warn('[TimeToDisplay] The `record` prop is deprecated. Use `ready` instead.'); + } + } + + // Subscribe FIRST so this component receives its own registration notify + // (and any peer notifications) on mount. + useEffect(() => { + if (!parentSpanId) { + return undefined; + } + return subscribe(kind, parentSpanId, force); + }, [kind, parentSpanId]); + + // Register on mount / when the active span changes; unregister on unmount. + useEffect(() => { + if (!parentSpanId) { + return undefined; + } + return registerCheckpoint(kind, parentSpanId, checkpointId, localReady); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [kind, parentSpanId, checkpointId]); + + // Propagate ready transitions to the registry. + useEffect(() => { + if (!parentSpanId) { + return; + } + updateCheckpoint(kind, parentSpanId, checkpointId, localReady); + }, [kind, parentSpanId, checkpointId, localReady]); + + if (!parentSpanId) { + return false; + } + return isAllReady(kind, parentSpanId); +} + function TimeToDisplay(props: { children?: React.ReactNode; initialDisplay?: boolean; diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts new file mode 100644 index 0000000000..78f2d7d0ff --- /dev/null +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -0,0 +1,127 @@ +import { + _resetTimeToDisplayCoordinator, + hasAnyCheckpoints, + isAllReady, + registerCheckpoint, + subscribe, + updateCheckpoint, +} from '../../src/js/tracing/timeToDisplayCoordinator'; + +const SPAN_FIRST = 'span-first'; +const SPAN_SECOND = 'span-second'; + +describe('timeToDisplayCoordinator', () => { + beforeEach(() => { + _resetTimeToDisplayCoordinator(); + }); + + test('empty registry is not ready', () => { + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); + }); + + test('single not-ready checkpoint blocks', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + }); + + test('single ready checkpoint resolves', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('all ready resolves; one not-ready blocks', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); + registerCheckpoint('ttfd', SPAN_FIRST, 'c', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + + updateCheckpoint('ttfd', SPAN_FIRST, 'c', true); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('late-registering not-ready checkpoint un-readies the aggregate', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + + registerCheckpoint('ttfd', SPAN_FIRST, 'c', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + }); + + test('unregistering the only blocking checkpoint resolves the aggregate', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + + unregisterB(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('unregistering the last checkpoint leaves aggregate not-ready', () => { + const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + + unregister(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); + }); + + test('different spans are independent', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + registerCheckpoint('ttfd', SPAN_SECOND, 'a', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + expect(isAllReady('ttfd', SPAN_SECOND)).toBe(false); + }); + + test('different kinds are independent', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + registerCheckpoint('ttid', SPAN_FIRST, 'a', false); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + expect(isAllReady('ttid', SPAN_FIRST)).toBe(false); + }); + + test('updateCheckpoint is a no-op for unknown id', () => { + const listener = jest.fn(); + subscribe('ttfd', SPAN_FIRST, listener); + updateCheckpoint('ttfd', SPAN_FIRST, 'nope', true); + expect(listener).not.toHaveBeenCalled(); + }); + + test('updateCheckpoint with same ready value does not notify', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + const listener = jest.fn(); + subscribe('ttfd', SPAN_FIRST, listener); + updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(listener).not.toHaveBeenCalled(); + }); + + test('subscribers are notified on register / update / unregister', () => { + const listener = jest.fn(); + subscribe('ttfd', SPAN_FIRST, listener); + + const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); + expect(listener).toHaveBeenCalledTimes(1); + + updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(listener).toHaveBeenCalledTimes(2); + + unregister(); + expect(listener).toHaveBeenCalledTimes(3); + }); + + test('unsubscribe stops further notifications', () => { + const listener = jest.fn(); + const unsubscribe = subscribe('ttfd', SPAN_FIRST, listener); + unsubscribe(); + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(listener).not.toHaveBeenCalled(); + }); + + test('subscribers on one span ignore changes on another span', () => { + const listener = jest.fn(); + subscribe('ttfd', SPAN_FIRST, listener); + registerCheckpoint('ttfd', SPAN_SECOND, 'a', true); + expect(listener).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx new file mode 100644 index 0000000000..5dd84da620 --- /dev/null +++ b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx @@ -0,0 +1,225 @@ +import type { Span } from '@sentry/core'; + +import { + getCurrentScope, + getGlobalScope, + getIsolationScope, + setCurrentClient, + spanToJSON, + startSpanManual, +} from '@sentry/core'; + +import * as mockWrapper from '../mockWrapper'; + +jest.mock('../../src/js/wrapper', () => mockWrapper); + +import * as mockedtimetodisplaynative from './mockedtimetodisplaynative'; + +jest.mock('../../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative); + +import { act, render } from '@testing-library/react-native'; +import * as React from 'react'; + +import { _resetTimeToDisplayCoordinator } from '../../src/js/tracing/timeToDisplayCoordinator'; +import { TimeToFullDisplay, TimeToInitialDisplay } from '../../src/js/tracing/timetodisplay'; +import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; +import { secondAgoTimestampMs } from '../testutils'; + +jest.mock('../../src/js/utils/environment', () => ({ + isWeb: jest.fn().mockReturnValue(false), + isTurboModuleEnabled: jest.fn().mockReturnValue(false), +})); + +const { getMockedOnDrawReportedProps, clearMockedOnDrawReportedProps } = mockedtimetodisplaynative; + +function tailHasFullDisplay(parentSpanId: string, mountedReporterCount: number): boolean { + const props = getMockedOnDrawReportedProps().filter(p => p.parentSpanId === parentSpanId); + const tail = props.slice(-mountedReporterCount); + return tail.some(p => p.fullDisplay === true); +} + +function tailHasInitialDisplay(parentSpanId: string, mountedReporterCount: number): boolean { + const props = getMockedOnDrawReportedProps().filter(p => p.parentSpanId === parentSpanId); + const tail = props.slice(-mountedReporterCount); + return tail.some(p => p.initialDisplay === true); +} + +jest.useFakeTimers({ advanceTimers: true, doNotFake: ['performance'] }); + +describe('TimeToDisplay multi-instance (`ready` prop)', () => { + beforeEach(() => { + clearMockedOnDrawReportedProps(); + _resetTimeToDisplayCoordinator(); + getCurrentScope().clear(); + getIsolationScope().clear(); + getGlobalScope().clear(); + const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); + const client = new TestClient(options); + setCurrentClient(client); + client.init(); + }); + + afterEach(() => { + jest.clearAllMocks(); + mockWrapper.NATIVE.enableNative = true; + }); + + test('legacy: single `record` instance behaves identically to today', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + activeSpan?.end(); + }); + }); + + test('two `ready={false}` instances do not emit', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render( + <> + + + , + ); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + activeSpan?.end(); + }); + }); + + test('two `ready` instances emit only when both are ready', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ a, b }: { a: boolean; b: boolean }) => ( + <> + + + + ); + + const tree = render(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('late-mounting `ready={false}` un-readies an already-ready aggregate', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ showLate, lateReady }: { showLate: boolean; lateReady: boolean }) => ( + <> + + {showLate ? : null} + + ); + + const tree = render(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('unmounting the only blocking checkpoint emits ready', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ showBlocker }: { showBlocker: boolean }) => ( + <> + + {showBlocker ? : null} + + ); + + const tree = render(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('mixed `record` + `ready`: both must be satisfied', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ rec, rdy }: { rec: boolean; rdy: boolean }) => ( + <> + + + + ); + + const tree = render(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('different active spans have independent registries', () => { + let firstSpanId = ''; + let secondSpanId = ''; + + startSpanManual({ name: 'Screen A', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + firstSpanId = spanToJSON(activeSpan!).span_id; + render(); + activeSpan?.end(); + }); + + clearMockedOnDrawReportedProps(); + + startSpanManual({ name: 'Screen B', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + secondSpanId = spanToJSON(activeSpan!).span_id; + render(); + expect(tailHasFullDisplay(secondSpanId, 1)).toBe(false); + activeSpan?.end(); + }); + + expect(firstSpanId).not.toEqual(secondSpanId); + }); + + test('TTID `ready` aggregates symmetrically', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ a, b }: { a: boolean; b: boolean }) => ( + <> + + + + ); + + const tree = render(); + expect(tailHasInitialDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + expect(tailHasInitialDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); +}); From f156953eb5c138aa0aac6794ed6f0cb61981be84 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 11:50:57 +0200 Subject: [PATCH 2/7] Changelog entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5837019405..e8d70c5366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ - Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076)) +### Features + + - Multi-instance `` / `` coordination ([#XXXX](https://github.com/getsentry/sentry-react-native/pulls/XXXX)) + - When a screen has multiple async data sources, you can now mount one `` per source — the TTID/TTFD will get recorded + only when all the sources report `ready`. + - The new `ready` prop is declarative and replaces the previously imperative `record` prop, which is now marked as deprecated. + ## 8.10.0 ### Features From cb268c8dc47b980a53e4a1200f9a3c7870a739f4 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 11:57:32 +0200 Subject: [PATCH 3/7] Added changelog PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8d70c5366..3a44e4eecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Features - - Multi-instance `` / `` coordination ([#XXXX](https://github.com/getsentry/sentry-react-native/pulls/XXXX)) + - Multi-instance `` / `` coordination ([#6090](https://github.com/getsentry/sentry-react-native/pull/6090) - When a screen has multiple async data sources, you can now mount one `` per source — the TTID/TTFD will get recorded only when all the sources report `ready`. - The new `ready` prop is declarative and replaces the previously imperative `record` prop, which is now marked as deprecated. From aa62b625034fc4e5d1a0dfe6b0d6a1ed0d942177 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 12:01:38 +0200 Subject: [PATCH 4/7] Updates the order of entries in CHANGELOG.md --- CHANGELOG.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a44e4eecf..df79b7edec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,16 +8,16 @@ ## Unreleased -### Fixes +### Features -- Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076)) +- Multi-instance `` / `` coordination ([#6090](https://github.com/getsentry/sentry-react-native/pull/6090) + - When a screen has multiple async data sources, you can now mount one `` per source — the TTID/TTFD will get recorded + only when all the sources report `ready`. + - The new `ready` prop is declarative and replaces the previously imperative `record` prop, which is now marked as deprecated. -### Features +### Fixes - - Multi-instance `` / `` coordination ([#6090](https://github.com/getsentry/sentry-react-native/pull/6090) - - When a screen has multiple async data sources, you can now mount one `` per source — the TTID/TTFD will get recorded - only when all the sources report `ready`. - - The new `ready` prop is declarative and replaces the previously imperative `record` prop, which is now marked as deprecated. +- Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076)) ## 8.10.0 From 0598e1bc833564dc0634b5d186a79ecd0254aa34 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 12:05:17 +0200 Subject: [PATCH 5/7] useRef instead of useId for React 17 compatibility --- packages/core/src/js/tracing/timetodisplay.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index f6bbb8119c..d248cda45b 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -13,7 +13,7 @@ import { startInactiveSpan, } from '@sentry/core'; import * as React from 'react'; -import { useEffect, useId, useReducer, useState } from 'react'; +import { useEffect, useReducer, useRef, useState } from 'react'; import type { NativeFramesResponse } from '../NativeRNSentry'; @@ -130,12 +130,23 @@ export function TimeToFullDisplay(props: TimeToDisplayProps): React.ReactElement * a checkpoint under the active span. The aggregate is ready if every * checkpoint reports ready. */ +/** + * Module-local counter used to mint stable, unique checkpoint ids per + * component instance without requiring React 18's `useId`. + */ +let nextCheckpointId = 0; + function useCoordinatedDisplay( kind: DisplayKind, parentSpanId: string | undefined, props: TimeToDisplayProps, ): boolean { - const checkpointId = useId(); + // Stable per-instance id. `useRef` is available since React 16.8. + const checkpointIdRef = useRef(null); + if (checkpointIdRef.current === null) { + checkpointIdRef.current = `cp-${nextCheckpointId++}`; + } + const checkpointId = checkpointIdRef.current; const [, force] = useReducer((x: number) => x + 1, 0); // `ready` takes precedence when both are provided. From a9937e97f6aebb05fe9f741f2fdbc6a79e118dd5 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 12:08:51 +0200 Subject: [PATCH 6/7] Use refs to only throw warnings once --- packages/core/src/js/tracing/timetodisplay.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index d248cda45b..f9b6b13149 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -152,14 +152,20 @@ function useCoordinatedDisplay( // `ready` takes precedence when both are provided. const localReady = props.ready !== undefined ? !!props.ready : !!props.record; - if (__DEV__) { + // Using refs here to only throw warnings once + const warnedRef = useRef(false); + useEffect(() => { + if (!__DEV__ || warnedRef.current) return; if (props.ready !== undefined && props.record !== undefined) { + warnedRef.current = true; debug.warn('[TimeToDisplay] Both `ready` and `record` were provided — ignoring `record`.'); } if (props.record !== undefined) { + warnedRef.current = true; debug.warn('[TimeToDisplay] The `record` prop is deprecated. Use `ready` instead.'); } - } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Subscribe FIRST so this component receives its own registration notify // (and any peer notifications) on mount. From a420720bb669fba0f9348d8d0b0cda636e1ee843 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Wed, 6 May 2026 14:47:46 +0200 Subject: [PATCH 7/7] An attempt to fix things --- CHANGELOG.md | 13 ++-- .../js/tracing/timeToDisplayCoordinator.ts | 67 +++++++++++++------ .../core/src/js/tracing/timetodisplay.tsx | 62 +++++++++++------ .../tracing/timeToDisplayCoordinator.test.ts | 26 +++++-- .../timetodisplay.multiinstance.test.tsx | 48 ++++++++++++- 5 files changed, 163 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df79b7edec..6ac49f2859 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,15 @@ ### Features -- Multi-instance `` / `` coordination ([#6090](https://github.com/getsentry/sentry-react-native/pull/6090) - - When a screen has multiple async data sources, you can now mount one `` per source — the TTID/TTFD will get recorded - only when all the sources report `ready`. - - The new `ready` prop is declarative and replaces the previously imperative `record` prop, which is now marked as deprecated. +- Multi-instance `` / `` coordination ([#6090](https://github.com/getsentry/sentry-react-native/pull/6090)) + - New `ready` prop. When a screen has multiple async data sources, mount one `` per source — TTID/TTFD is recorded only when every instance reports `ready === true`. + ```tsx + + + // TTFD fires when both are ready. + ``` + - The existing `record` prop is **unchanged** and continues to behave exactly as before — instances using `record` are independent and do not gate or get gated by `ready` peers. Existing apps require no migration. + - `record` is now deprecated in favor of `ready`. Migrating to `ready` is a one-line rename that opts the instance into multi-instance coordination. `record` will be removed in the next major version. ### Fixes diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index 179bf1ff17..0be223d542 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -9,6 +9,13 @@ type Listener = () => void; interface SpanRegistry { checkpoints: Map; listeners: Set; + /** + * Last-observed aggregate ready state. Used to avoid waking subscribers when + * a checkpoint change does not flip the aggregate — the dominant lifecycle + * pattern is "all checkpoints register as not-ready, then flip to ready over + * time", and only the final flip needs to notify. + */ + aggregateReady: boolean; } const TTID = 'ttid'; @@ -27,13 +34,43 @@ function getOrCreate(kind: DisplayKind, parentSpanId: string): SpanRegistry { if (!entry) { entry = { checkpoints: new Map(), - listeners: new Set() + listeners: new Set(), + aggregateReady: false, }; map.set(parentSpanId, entry); } return entry; } +function computeAggregate(entry: SpanRegistry): boolean { + if (entry.checkpoints.size === 0) { + return false; + } + for (const cp of entry.checkpoints.values()) { + if (!cp.ready) { + return false; + } + } + return true; +} + +/** + * Recompute the aggregate; if it flipped, update the cached value and notify. + * No-op when the aggregate is unchanged — this is what avoids the O(N²) + * notify-storm when many checkpoints register/update without crossing the + * aggregate boundary. + */ +function reevaluate(entry: SpanRegistry): void { + const next = computeAggregate(entry); + if (next === entry.aggregateReady) { + return; + } + entry.aggregateReady = next; + for (const listener of entry.listeners) { + listener(); + } +} + function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { if (entry.checkpoints.size === 0 && entry.listeners.size === 0) { registries[kind].delete(parentSpanId); @@ -51,7 +88,7 @@ export function registerCheckpoint( ): () => void { const entry = getOrCreate(kind, parentSpanId); entry.checkpoints.set(checkpointId, { ready }); - notify(entry); + reevaluate(entry); return () => { const e = registries[kind].get(parentSpanId); @@ -59,7 +96,7 @@ export function registerCheckpoint( return; } if (e.checkpoints.delete(checkpointId)) { - notify(e); + reevaluate(e); } performCleanup(kind, parentSpanId, e); }; @@ -80,23 +117,16 @@ export function updateCheckpoint( return; } cp.ready = ready; - notify(entry); + reevaluate(entry); } /** * True if at least one checkpoint is registered AND all checkpoints are ready. + * Reads the cached aggregate — O(1). */ export function isAllReady(kind: DisplayKind, parentSpanId: string): boolean { const entry = registries[kind].get(parentSpanId); - if (!entry || entry.checkpoints.size === 0) { - return false; - } - for (const cp of entry.checkpoints.values()) { - if (!cp.ready) { - return false; - } - } - return true; + return !!entry && entry.aggregateReady; } /** @@ -108,8 +138,9 @@ export function hasAnyCheckpoints(kind: DisplayKind, parentSpanId: string): bool } /** - * Subscribe to any checkpoint state change for a given span. The listener is - * called synchronously after each register/update/unregister event. + * Subscribe to aggregate-ready transitions for a given span. The listener is + * called only when the aggregate flips, not on every individual checkpoint + * change. */ export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Listener): () => void { const entry = getOrCreate(kind, parentSpanId); @@ -124,12 +155,6 @@ export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Lis }; } -function notify(entry: SpanRegistry): void { - for (const listener of entry.listeners) { - listener(); - } -} - /** * Test-only. Clears all coordinator state. */ diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index f9b6b13149..bc9a330298 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -126,13 +126,26 @@ export function TimeToFullDisplay(props: TimeToDisplayProps): React.ReactElement } /** - * Every `` / `` instance registers as - * a checkpoint under the active span. The aggregate is ready if every - * checkpoint reports ready. - */ -/** - * Module-local counter used to mint stable, unique checkpoint ids per - * component instance without requiring React 18's `useId`. + * Resolves the boolean passed to the underlying native draw reporter. + * + * Two semantically-distinct modes preserve backward compatibility: + * + * 1. **Legacy (`record`)** — the component is independent. The reporter + * receives `!!props.record` directly. Multiple `record`-only peers don't + * coordinate; the native side resolves them via last-write-wins, exactly + * as before this change. + * + * 2. **Registry (`ready`)** — the component is a checkpoint. It registers + * under the active span and the reporter receives the per-span aggregate. + * Multiple `ready` peers coordinate: every one of them must be ready + * before any of their reporters emits true. + * + * Mode is selected per-instance: `ready !== undefined` opts into registry + * mode. A bare `` (no props) is legacy mode with + * `record=false` — a no-op, same as today. + * + * `ready` and `record` will be unified into one prop in the next major when + * `record` is removed. */ let nextCheckpointId = 0; @@ -149,18 +162,17 @@ function useCoordinatedDisplay( const checkpointId = checkpointIdRef.current; const [, force] = useReducer((x: number) => x + 1, 0); - // `ready` takes precedence when both are provided. - const localReady = props.ready !== undefined ? !!props.ready : !!props.record; + const useRegistry = props.ready !== undefined; + const localReady = useRegistry ? !!props.ready : !!props.record; - // Using refs here to only throw warnings once + // Emit deprecation / conflict warnings once per component instance. const warnedRef = useRef(false); useEffect(() => { if (!__DEV__ || warnedRef.current) return; if (props.ready !== undefined && props.record !== undefined) { warnedRef.current = true; debug.warn('[TimeToDisplay] Both `ready` and `record` were provided — ignoring `record`.'); - } - if (props.record !== undefined) { + } else if (props.record !== undefined) { warnedRef.current = true; debug.warn('[TimeToDisplay] The `record` prop is deprecated. Use `ready` instead.'); } @@ -168,34 +180,44 @@ function useCoordinatedDisplay( }, []); // Subscribe FIRST so this component receives its own registration notify - // (and any peer notifications) on mount. + // (and any peer notifications) on mount. Only registry-mode components + // need peer notifications. useEffect(() => { - if (!parentSpanId) { + if (!parentSpanId || !useRegistry) { return undefined; } return subscribe(kind, parentSpanId, force); - }, [kind, parentSpanId]); + }, [kind, parentSpanId, useRegistry]); // Register on mount / when the active span changes; unregister on unmount. + // Legacy-mode components do not register — they are independent and don't + // gate or get gated by peers. useEffect(() => { - if (!parentSpanId) { + if (!parentSpanId || !useRegistry) { return undefined; } return registerCheckpoint(kind, parentSpanId, checkpointId, localReady); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [kind, parentSpanId, checkpointId]); + }, [kind, parentSpanId, useRegistry, checkpointId]); - // Propagate ready transitions to the registry. + // Propagate ready transitions to the registry. Legacy-mode components + // skip this — they propagate their value directly via the returned boolean. useEffect(() => { - if (!parentSpanId) { + if (!parentSpanId || !useRegistry) { return; } updateCheckpoint(kind, parentSpanId, checkpointId, localReady); - }, [kind, parentSpanId, checkpointId, localReady]); + }, [kind, parentSpanId, useRegistry, checkpointId, localReady]); if (!parentSpanId) { return false; } + // Legacy: propagate the local `record` value directly. Native last-wins + // resolves multi-instance ordering exactly as before. + if (!useRegistry) { + return localReady; + } + // Registry: gated on the per-span aggregate. return isAllReady(kind, parentSpanId); } diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index 78f2d7d0ff..2c17c73868 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -96,18 +96,34 @@ describe('timeToDisplayCoordinator', () => { expect(listener).not.toHaveBeenCalled(); }); - test('subscribers are notified on register / update / unregister', () => { + test('subscribers are notified only on aggregate-ready flips', () => { const listener = jest.fn(); subscribe('ttfd', SPAN_FIRST, listener); const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); - expect(listener).toHaveBeenCalledTimes(1); - + expect(listener).toHaveBeenCalledTimes(0); updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(listener).toHaveBeenCalledTimes(1); + unregister(); expect(listener).toHaveBeenCalledTimes(2); + }); - unregister(); - expect(listener).toHaveBeenCalledTimes(3); + test('non-flipping checkpoint changes do not wake subscribers (storm avoidance)', () => { + const listener = jest.fn(); + subscribe('ttfd', SPAN_FIRST, listener); + + for (let i = 0; i < 10; i++) { + registerCheckpoint('ttfd', SPAN_FIRST, `cp-${i}`, false); + } + expect(listener).toHaveBeenCalledTimes(0); + + for (let i = 0; i < 9; i++) { + updateCheckpoint('ttfd', SPAN_FIRST, `cp-${i}`, true); + } + expect(listener).toHaveBeenCalledTimes(0); + + updateCheckpoint('ttfd', SPAN_FIRST, 'cp-9', true); + expect(listener).toHaveBeenCalledTimes(1); }); test('unsubscribe stops further notifications', () => { diff --git a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx index 5dd84da620..b9e8f43933 100644 --- a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx +++ b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx @@ -156,7 +156,11 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { }); }); - test('mixed `record` + `ready`: both must be satisfied', () => { + test('mixed `record` + `ready`: legacy `record` is independent, `ready` peers coordinate', () => { + // Backward compat: `record`-only instances do not register as checkpoints + // and are not gated by `ready` peers. They emit `fullDisplay` directly + // from their own prop, exactly as before this change. `ready` peers gate + // each other via the registry. startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -167,12 +171,16 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); + // record=true fires independently; ready=false blocks the ready reporter. + // The tail reflects: [record:true, ready:false] → fullDisplay=true present. const tree = render(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + // record=false stops emitting; ready=true now fires. act(() => tree.rerender()); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + // Both fire. act(() => tree.rerender()); expect(tailHasFullDisplay(spanId, 2)).toBe(true); @@ -180,6 +188,40 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { }); }); + test('legacy: bare does not block `ready` peers', () => { + // Backward compat for layout-placeholder usage. A bare component with + // neither prop is a no-op (legacy `record=false`). + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render( + <> + + + , + ); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + activeSpan?.end(); + }); + }); + + test('legacy: two `record` peers fire independently (no coordination)', () => { + // Backward compat: pre-change behavior was last-write-wins on the native + // side. record-only peers must continue to fire independently. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render( + <> + + + , + ); + // The record=true reporter fires; record=false does not. fullDisplay=true + // present in the tail. + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + activeSpan?.end(); + }); + }); + test('different active spans have independent registries', () => { let firstSpanId = ''; let secondSpanId = '';