From 9b9178c215048b193bd74ca0bcfd3c5074f57aa4 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Tue, 5 May 2026 11:46:32 +0200 Subject: [PATCH 01/14] 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 02/14] 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 03/14] 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 04/14] 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 05/14] 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 06/14] 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 07/14] 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 = ''; From 9ef82b84ec34856389b8a60d9c2325688b8a9bf0 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 10:22:45 +0200 Subject: [PATCH 08/14] Fixes --- .../js/tracing/timeToDisplayCoordinator.ts | 64 ++++++++++++++++++- .../tracing/timeToDisplayCoordinator.test.ts | 33 +++++++++- .../timetodisplay.multiinstance.test.tsx | 36 ++++++++++- 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index 0be223d542..e9106b3bb0 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -16,6 +16,13 @@ interface SpanRegistry { * time", and only the final flip needs to notify. */ aggregateReady: boolean; + /** + * Checkpoint ids whose components have unmounted but are kept in the + * registry to prevent a premature aggregate flip (sole-blocker safeguard). + * They participate in `computeAggregate` but are excluded from the + * "live work" count in `performCleanup`. + */ + sticky: Set; } const TTID = 'ttid'; @@ -36,6 +43,7 @@ function getOrCreate(kind: DisplayKind, parentSpanId: string): SpanRegistry { checkpoints: new Map(), listeners: new Set(), aggregateReady: false, + sticky: new Set(), }; map.set(parentSpanId, entry); } @@ -71,12 +79,56 @@ function reevaluate(entry: SpanRegistry): void { } } +/** + * Delete the registry entry once there is no live work attached to it. + * + * "Live work" means either subscribed listeners (registry-mode components + * still mounted) or non-sticky checkpoints (still-mounted registrations). + * Sticky checkpoints (kept after a not-ready unmount; see `unregister`) + * exist only to prevent premature aggregate flips while the screen is still + * mounted; once every live counterpart is gone, they are orphaned and safe + * to drop along with the entry. + */ function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { - if (entry.checkpoints.size === 0 && entry.listeners.size === 0) { + const liveCheckpoints = entry.checkpoints.size - entry.sticky.size; + if (liveCheckpoints === 0 && entry.listeners.size === 0) { registries[kind].delete(parentSpanId); } } +/** + * True iff removing `checkpointId` from `entry` would flip the aggregate + * from false to true — i.e., the checkpoint is the sole blocker. + * + * Used to detect the premature-fire scenario where a not-ready checkpoint + * unmounts while every other checkpoint is ready: deleting it would let the + * aggregate flip to true and immediately record TTFD/TTID, even though the + * unmounting source never actually became ready. + */ +function isSoleBlocker(entry: SpanRegistry, checkpointId: string): boolean { + if (entry.aggregateReady) { + return false; + } + if (entry.checkpoints.size <= 1) { + // Removing the only checkpoint leaves the registry empty, which yields + // aggregate=false (per `computeAggregate`). No flip. + return false; + } + const cp = entry.checkpoints.get(checkpointId); + if (!cp || cp.ready) { + return false; + } + for (const [id, other] of entry.checkpoints) { + if (id === checkpointId) { + continue; + } + if (!other.ready) { + return false; + } + } + return true; +} + /** * Register a checkpoint under (kind, parentSpanId). Returns an unregister fn. */ @@ -95,7 +147,17 @@ export function registerCheckpoint( if (!e) { return; } + // If this checkpoint is the sole blocker, removing it would flip the + // aggregate to true and prematurely fire TTFD/TTID even though the + // unmounting source never became ready. Keep the checkpoint sticky; + // it gets cleared when the screen fully unmounts. + if (isSoleBlocker(e, checkpointId)) { + e.sticky.add(checkpointId); + performCleanup(kind, parentSpanId, e); + return; + } if (e.checkpoints.delete(checkpointId)) { + e.sticky.delete(checkpointId); reevaluate(e); } performCleanup(kind, parentSpanId, e); diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index 2c17c73868..100fd2c3ff 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -49,12 +49,43 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); }); - test('unregistering the only blocking checkpoint resolves the aggregate', () => { + test('unregistering the sole blocker keeps the aggregate not-ready (sticky)', () => { + // A not-ready checkpoint that unmounts while it is the sole blocker is + // kept in the registry to prevent a premature aggregate flip. Otherwise + // a conditionally-rendered loading section that disappears before its + // data resolves would silently record an incomplete display. 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(false); + // The sticky 'b' is still tracked so a subsequent component on the same + // span continues to see it as a blocker. + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('unregistering a non-sole-blocker not-ready checkpoint removes it normally', () => { + // When other not-ready checkpoints are still present, removing one + // would not flip the aggregate, so the sticky safeguard does not apply. + registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); + const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); + + unregisterB(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + // 'a' remains, 'b' is gone. + updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('unregistering a ready checkpoint never goes sticky', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); + const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); + + unregisterB(); + // 'a' is still blocking; 'b' was ready so removing it is safe. + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); diff --git a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx index b9e8f43933..3f4ed26440 100644 --- a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx +++ b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx @@ -135,7 +135,11 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { }); }); - test('unmounting the only blocking checkpoint emits ready', () => { + test('unmounting the sole blocker does NOT emit ready (sticky safeguard)', () => { + // A conditionally-rendered loading section that disappears before its + // data resolves must not trick TTFD into firing as if the screen were + // fully displayed. The sole-blocker checkpoint is kept sticky in the + // registry, so its unmount cannot flip the aggregate to true. startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -150,6 +154,36 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { expect(tailHasFullDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 1)).toBe(false); + + activeSpan?.end(); + }); + }); + + test('unmounting a non-sole-blocker resolves the aggregate when remaining peers are ready', () => { + // The sticky safeguard only applies to *sole* blockers. If other + // not-ready peers exist, an unmount can't flip the aggregate to true, + // so it removes normally. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ aReady, showB }: { aReady: boolean; showB: boolean }) => ( + <> + + {showB ? : null} + + ); + + const tree = render(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + // Unmount B while A is also not-ready: not a sole-blocker case, B + // removes normally; aggregate still blocked by A. + act(() => tree.rerender()); + expect(tailHasFullDisplay(spanId, 1)).toBe(false); + + // Now flip A to ready: aggregate flips, A's reporter emits. + act(() => tree.rerender()); expect(tailHasFullDisplay(spanId, 1)).toBe(true); activeSpan?.end(); From 273758a2b95371f0421e161957d26eb9043aefca Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 11:00:45 +0200 Subject: [PATCH 09/14] Fixes --- CHANGELOG.md | 1 + .../integrations/timeToDisplayIntegration.ts | 7 + .../js/tracing/timeToDisplayCoordinator.ts | 129 ++++++++++++++++-- .../core/src/js/tracing/timetodisplay.tsx | 24 +++- .../tracing/timeToDisplayCoordinator.test.ts | 86 ++++++++++++ .../timetodisplay.multiinstance.test.tsx | 102 ++++++++++++++ 6 files changed, 334 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ac49f2859..fa7082ea7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ ``` - 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. + - Mounting a not-yet-ready peer on the next render commit (typical `useEffect` → `setState` → child mount waves) cancels any pending fire from an earlier-ready peer, preventing premature TTFD recording. Up-flips of the aggregate are deferred by one macrotask to absorb same-task peer mounts; down-flips are immediate. ### Fixes diff --git a/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts b/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts index b6214c1542..e3ca401b3b 100644 --- a/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts +++ b/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts @@ -4,6 +4,7 @@ import { debug } from '@sentry/core'; import { NATIVE } from '../../wrapper'; import { UI_LOAD_FULL_DISPLAY, UI_LOAD_INITIAL_DISPLAY } from '../ops'; +import { clearSpan as clearTimeToDisplayCoordinatorSpan } from '../timeToDisplayCoordinator'; import { SPAN_ORIGIN_AUTO_UI_TIME_TO_DISPLAY, SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY } from '../origin'; import { getReactNavigationIntegration } from '../reactnavigation'; import { SEMANTIC_ATTRIBUTE_ROUTE_HAS_BEEN_SEEN } from '../semanticAttributes'; @@ -86,6 +87,12 @@ export const timeToDisplayIntegration = (): Integration => { event.timestamp = newTransactionEndTimestampSeconds; } + // Drop the per-span coordinator state now that we've read the native + // ttid/ttfd values. Prevents the module-level registries from + // accumulating entries for screens that outlive their span (keep-alive, + // idle-timeout discarded transactions, etc.). + clearTimeToDisplayCoordinatorSpan(rootSpanId); + return event; }, }; diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index e9106b3bb0..5d6253894a 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -1,6 +1,27 @@ /** * Coordinator for multi-instance `` / `` * components on a single screen (active span). + * + * The aggregate "ready" state exposed via `isAllReady` is *deferred* on its + * way up: when the raw aggregate flips false→true, we schedule the public + * value to flip on the next macrotask. Down-flips (true→false) are + * immediate and cancel any pending up-flip. + * + * Why: in React 18, a typical "parent renders → parent useEffect setState + * → child mounts on next commit" wave executes synchronously inside one + * event-loop task. A `setTimeout(0)` reliably runs after that whole wave, + * so cross-commit-but-same-task peer registrations are absorbed before the + * coordinator declares itself ready. Without the defer, a header that + * registers and is alone-and-ready would emit `fullDisplay=true` + * immediately, the native reporter would fire on the next draw, and a + * sibling sidebar mounting on the next commit could only un-ready the + * aggregate after the (now stuck) native timestamp has already been + * recorded. + * + * The defer does NOT cover arbitrary-async deferred mounting (e.g. mount a + * checkpoint after a fetch resolves). That class of usage is documented + * against — the recommended pattern is to mount all checkpoints at screen + * mount with `ready=false` and flip them as data arrives. */ type Checkpoint = { ready: boolean }; @@ -10,12 +31,20 @@ 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. + * Stable, deferred view of the aggregate exposed via `isAllReady`. Lags + * raw `computeAggregate` on up-flips by `READY_DEFER_MS`, immediate on + * down-flips. 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; + /** + * Pending up-flip timer. When non-null, an up-flip is scheduled but has + * not yet been applied to `aggregateReady`. Cleared either when the timer + * fires or when an intervening change cancels the pending up-flip. + */ + pendingUpFlip: ReturnType | null; /** * Checkpoint ids whose components have unmounted but are kept in the * registry to prevent a premature aggregate flip (sole-blocker safeguard). @@ -25,6 +54,12 @@ interface SpanRegistry { sticky: Set; } +/** + * Defer applied to up-flips. Zero macrotask is enough to absorb a same-task + * cascade of useEffect-driven child mounts in React 18. + */ +const READY_DEFER_MS = 0; + const TTID = 'ttid'; const TTFD = 'ttfd'; @@ -43,6 +78,7 @@ function getOrCreate(kind: DisplayKind, parentSpanId: string): SpanRegistry { checkpoints: new Map(), listeners: new Set(), aggregateReady: false, + pendingUpFlip: null, sticky: new Set(), }; map.set(parentSpanId, entry); @@ -50,6 +86,13 @@ function getOrCreate(kind: DisplayKind, parentSpanId: string): SpanRegistry { return entry; } +function cancelPendingUpFlip(entry: SpanRegistry): void { + if (entry.pendingUpFlip !== null) { + clearTimeout(entry.pendingUpFlip); + entry.pendingUpFlip = null; + } +} + function computeAggregate(entry: SpanRegistry): boolean { if (entry.checkpoints.size === 0) { return false; @@ -63,17 +106,47 @@ function computeAggregate(entry: SpanRegistry): boolean { } /** - * 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. + * Recompute the raw aggregate and reconcile it with the cached + * `aggregateReady`. Up-flips are deferred to absorb same-task peer mounts; + * down-flips are immediate and cancel any pending up-flip. + * + * Transition matrix (raw, stable) → action: + * (false, false): no-op; cancel pending up-flip if any (it became stale). + * (true, true): no-op; cancel pending up-flip if any. + * (false, true): immediate down-flip + notify; cancel pending up-flip. + * (true, false): schedule up-flip if not already pending. */ function reevaluate(entry: SpanRegistry): void { - const next = computeAggregate(entry); - if (next === entry.aggregateReady) { + const raw = computeAggregate(entry); + + if (raw === entry.aggregateReady) { + cancelPendingUpFlip(entry); return; } - entry.aggregateReady = next; + + if (!raw) { + cancelPendingUpFlip(entry); + entry.aggregateReady = false; + notifyListeners(entry); + return; + } + + // raw=true, stable=false: schedule deferred up-flip. + if (entry.pendingUpFlip !== null) { + return; + } + entry.pendingUpFlip = setTimeout(() => { + entry.pendingUpFlip = null; + // Re-check on fire — a peer may have un-readied between schedule and now. + if (!computeAggregate(entry) || entry.aggregateReady) { + return; + } + entry.aggregateReady = true; + notifyListeners(entry); + }, READY_DEFER_MS); +} + +function notifyListeners(entry: SpanRegistry): void { for (const listener of entry.listeners) { listener(); } @@ -92,6 +165,7 @@ function reevaluate(entry: SpanRegistry): void { function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { const liveCheckpoints = entry.checkpoints.size - entry.sticky.size; if (liveCheckpoints === 0 && entry.listeners.size === 0) { + cancelPendingUpFlip(entry); registries[kind].delete(parentSpanId); } } @@ -217,10 +291,39 @@ export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Lis }; } +/** + * Drop coordinator state for `parentSpanId` across both kinds. + * + * Called by the time-to-display integration once a transaction has been + * processed, since the per-span coordinator state is no longer relevant + * after the native draw timestamps have been read. Without this hook, + * entries for screens that stay mounted past the end of their span + * (React Navigation keep-alive, idle-timeout discarded transactions, + * etc.) would accumulate in the module-level registries. + * + * Components that are still subscribed when their span is cleared remain + * functional: their next interaction recreates the entry under the same + * (now stale) parentSpanId. Since the integration has already read the + * native ttid/ttfd values for that span, any subsequent fires are inert. + */ +export function clearSpan(parentSpanId: string): void { + for (const kind of [TTID, TTFD] as const) { + const entry = registries[kind].get(parentSpanId); + if (entry) { + cancelPendingUpFlip(entry); + registries[kind].delete(parentSpanId); + } + } +} + /** * Test-only. Clears all coordinator state. */ export function _resetTimeToDisplayCoordinator(): void { - registries.ttid.clear(); - registries.ttfd.clear(); + for (const kind of [TTID, TTFD] as const) { + for (const entry of registries[kind].values()) { + cancelPendingUpFlip(entry); + } + registries[kind].clear(); + } } diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index bc9a330298..61153199e4 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -189,6 +189,13 @@ function useCoordinatedDisplay( return subscribe(kind, parentSpanId, force); }, [kind, parentSpanId, useRegistry]); + // Tracks whether this component's checkpoint is currently registered with + // the coordinator. Used to detect the pre-register render so we don't + // inherit a peer's ready=true verdict before our own checkpoint exists in + // the aggregate. Reset on unregister so re-mount under a new active span + // re-clamps correctly. + const registeredRef = useRef(false); + // 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. @@ -196,7 +203,12 @@ function useCoordinatedDisplay( if (!parentSpanId || !useRegistry) { return undefined; } - return registerCheckpoint(kind, parentSpanId, checkpointId, localReady); + const unregister = registerCheckpoint(kind, parentSpanId, checkpointId, localReady); + registeredRef.current = true; + return () => { + registeredRef.current = false; + unregister(); + }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [kind, parentSpanId, useRegistry, checkpointId]); @@ -217,7 +229,15 @@ function useCoordinatedDisplay( if (!useRegistry) { return localReady; } - // Registry: gated on the per-span aggregate. + // Registry, pre-register render: our checkpoint is not yet in the + // registry, so `isAllReady` reflects only peer state. Combine it with our + // own `localReady` so we never inherit a peer's true verdict before our + // own checkpoint has had a chance to report. + if (!registeredRef.current) { + return localReady && isAllReady(kind, parentSpanId); + } + // Registry, post-register: gated on the per-span aggregate, which now + // includes our checkpoint. return isAllReady(kind, parentSpanId); } diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index 100fd2c3ff..d4e614daaa 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -1,5 +1,6 @@ import { _resetTimeToDisplayCoordinator, + clearSpan, hasAnyCheckpoints, isAllReady, registerCheckpoint, @@ -10,11 +11,21 @@ import { const SPAN_FIRST = 'span-first'; const SPAN_SECOND = 'span-second'; +/** Flush the coordinator's deferred up-flip timer. */ +function flushDefer(): void { + jest.runOnlyPendingTimers(); +} + describe('timeToDisplayCoordinator', () => { beforeEach(() => { + jest.useFakeTimers(); _resetTimeToDisplayCoordinator(); }); + afterEach(() => { + jest.useRealTimers(); + }); + test('empty registry is not ready', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); @@ -27,6 +38,7 @@ describe('timeToDisplayCoordinator', () => { test('single ready checkpoint resolves', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); @@ -37,12 +49,14 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); updateCheckpoint('ttfd', SPAN_FIRST, 'c', true); + flushDefer(); 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); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); registerCheckpoint('ttfd', SPAN_FIRST, 'c', false); @@ -75,6 +89,7 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); // 'a' remains, 'b' is gone. updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); @@ -86,11 +101,13 @@ describe('timeToDisplayCoordinator', () => { // 'a' is still blocking; 'b' was ready so removing it is safe. expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); test('unregistering the last checkpoint leaves aggregate not-ready', () => { const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); unregister(); @@ -101,6 +118,7 @@ describe('timeToDisplayCoordinator', () => { test('different spans are independent', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); registerCheckpoint('ttfd', SPAN_SECOND, 'a', false); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); expect(isAllReady('ttfd', SPAN_SECOND)).toBe(false); }); @@ -108,6 +126,7 @@ describe('timeToDisplayCoordinator', () => { test('different kinds are independent', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); registerCheckpoint('ttid', SPAN_FIRST, 'a', false); + flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); expect(isAllReady('ttid', SPAN_FIRST)).toBe(false); }); @@ -134,6 +153,7 @@ describe('timeToDisplayCoordinator', () => { const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); expect(listener).toHaveBeenCalledTimes(0); updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); expect(listener).toHaveBeenCalledTimes(1); unregister(); expect(listener).toHaveBeenCalledTimes(2); @@ -154,6 +174,7 @@ describe('timeToDisplayCoordinator', () => { expect(listener).toHaveBeenCalledTimes(0); updateCheckpoint('ttfd', SPAN_FIRST, 'cp-9', true); + flushDefer(); expect(listener).toHaveBeenCalledTimes(1); }); @@ -171,4 +192,69 @@ describe('timeToDisplayCoordinator', () => { registerCheckpoint('ttfd', SPAN_SECOND, 'a', true); expect(listener).not.toHaveBeenCalled(); }); + + test('up-flip is deferred so a same-task late-mounting peer can cancel it', () => { + // The defining race: header registers ready=true alone, sidebar mounts + // a tick later (e.g. parent useEffect setState→child mount). Without the + // defer, header would fire instantly. With the defer, sidebar's + // registration cancels the pending up-flip before it fires. + registerCheckpoint('ttfd', SPAN_FIRST, 'header', true); + // Aggregate is *raw* ready, but `isAllReady` (deferred view) is still false. + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + + // Same-task: sidebar mounts before the timer macrotask runs. + registerCheckpoint('ttfd', SPAN_FIRST, 'sidebar', false); + + flushDefer(); + // Aggregate must NOT have flipped — the defer protected us. + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + + // Sidebar resolves — now we get a real ready transition. + updateCheckpoint('ttfd', SPAN_FIRST, 'sidebar', true); + flushDefer(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + }); + + test('down-flip is immediate (cancels pending up-flip and not deferred itself)', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + flushDefer(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + + registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); + // No flushDefer: down-flip is immediate. + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + }); + + test('clearSpan drops all coordinator state for a span', () => { + // Simulates the integration calling clearSpan after popTimeToDisplayFor + // returns. Prevents the registries from accumulating entries for screens + // that outlive their span (keep-alive, idle-timeout discarded txns, etc.). + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + registerCheckpoint('ttid', SPAN_FIRST, 'a', true); + registerCheckpoint('ttfd', SPAN_SECOND, 'a', true); + flushDefer(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); + flushDefer(); + expect(isAllReady('ttid', SPAN_FIRST)).toBe(true); + flushDefer(); + expect(isAllReady('ttfd', SPAN_SECOND)).toBe(true); + + clearSpan(SPAN_FIRST); + + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); + expect(hasAnyCheckpoints('ttid', SPAN_FIRST)).toBe(false); + // Other spans untouched. + flushDefer(); + expect(isAllReady('ttfd', SPAN_SECOND)).toBe(true); + }); + + test('clearSpan also drops sticky checkpoints', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); + const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); + unregisterB(); // becomes sticky + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(true); + + clearSpan(SPAN_FIRST); + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); + }); }); diff --git a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx index 3f4ed26440..d4b5423016 100644 --- a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx +++ b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx @@ -32,6 +32,16 @@ jest.mock('../../src/js/utils/environment', () => ({ const { getMockedOnDrawReportedProps, clearMockedOnDrawReportedProps } = mockedtimetodisplaynative; +/** + * Flush the coordinator's deferred up-flip timer + any consequent React + * re-renders. Wrapped in act() so React applies the resulting state updates. + */ +function flushReadyDefer(): void { + act(() => { + jest.runOnlyPendingTimers(); + }); +} + function tailHasFullDisplay(parentSpanId: string, mountedReporterCount: number): boolean { const props = getMockedOnDrawReportedProps().filter(p => p.parentSpanId === parentSpanId); const tail = props.slice(-mountedReporterCount); @@ -68,6 +78,7 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; render(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 1)).toBe(true); activeSpan?.end(); }); @@ -82,6 +93,7 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { , ); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); activeSpan?.end(); }); @@ -99,12 +111,15 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); const tree = render(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); activeSpan?.end(); @@ -123,12 +138,15 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); const tree = render(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 1)).toBe(true); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); activeSpan?.end(); @@ -151,9 +169,11 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); const tree = render(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 1)).toBe(false); activeSpan?.end(); @@ -175,15 +195,18 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); const tree = render(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(false); // Unmount B while A is also not-ready: not a sole-blocker case, B // removes normally; aggregate still blocked by A. act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 1)).toBe(false); // Now flip A to ready: aggregate flips, A's reporter emits. act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 1)).toBe(true); activeSpan?.end(); @@ -208,14 +231,17 @@ 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(); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); // record=false stops emitting; ready=true now fires. act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); // Both fire. act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); activeSpan?.end(); @@ -233,11 +259,83 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { , ); + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); activeSpan?.end(); }); }); + test('late-mounting `ready={false}` peer does not inherit existing peer’s ready=true (pre-register clamp)', () => { + // Bug class: a fresh component on its first render calls `isAllReady` + // before its useEffect has registered its checkpoint. If a peer was + // already registered ready, the new instance would see aggregate=true + // and emit `fullDisplay=true` from its own reporter on its very first + // render — a premature fire whose timestamp would overwrite the (also + // premature) earlier one via native last-wins. + // + // The clamp `localReady && isAllReady` guarantees: if the new + // instance's local prop is false, it can never emit true on its first + // render even when peers are already ready. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ showLate }: { showLate: boolean }) => ( + <> + + {showLate ? : null} + + ); + + const tree = render(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + // Mount the late ready=false peer. Its very first render must NOT emit + // true even though aggregate is currently true (peer A is ready). + act(() => tree.rerender()); + // After both renders settle, neither reporter should have its current + // prop set to true. + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + activeSpan?.end(); + }); + }); + + test('same-task wave: header alone-and-ready does not fire when sibling mounts on next commit', () => { + // Reviewer's scenario: a header that loads instantly would otherwise + // finalize TTFD before a sibling that mounts a tick later registers as + // not-ready. The coordinator's deferred up-flip catches this race as + // long as the late mount happens within the same event-loop task + // (the typical "parent useEffect setState → child mount" wave). + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = (): React.ReactElement => { + const [showSidebar, setShowSidebar] = React.useState(false); + React.useEffect(() => { + setShowSidebar(true); + }, []); + return ( + <> + + {showSidebar ? : null} + + ); + }; + + render(); + // Both the synchronous header mount and the deferred-via-useEffect + // sidebar mount have completed; flushing the up-flip timer must NOT + // produce a fullDisplay=true emission, because the sidebar is now a + // registered blocker. + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + 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. @@ -251,6 +349,7 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); // The record=true reporter fires; record=false does not. fullDisplay=true // present in the tail. + flushReadyDefer(); expect(tailHasFullDisplay(spanId, 2)).toBe(true); activeSpan?.end(); }); @@ -271,6 +370,7 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { startSpanManual({ name: 'Screen B', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { secondSpanId = spanToJSON(activeSpan!).span_id; render(); + flushReadyDefer(); expect(tailHasFullDisplay(secondSpanId, 1)).toBe(false); activeSpan?.end(); }); @@ -290,9 +390,11 @@ describe('TimeToDisplay multi-instance (`ready` prop)', () => { ); const tree = render(); + flushReadyDefer(); expect(tailHasInitialDisplay(spanId, 2)).toBe(false); act(() => tree.rerender()); + flushReadyDefer(); expect(tailHasInitialDisplay(spanId, 2)).toBe(true); activeSpan?.end(); From e6b6c079de24c7f4e5f8f10cf3a7ce928a98fd46 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 11:54:08 +0200 Subject: [PATCH 10/14] Fixes --- CHANGELOG.md | 9 +- .../integrations/timeToDisplayIntegration.ts | 4 - .../js/tracing/timeToDisplayCoordinator.ts | 146 ++----- .../core/src/js/tracing/timetodisplay.tsx | 75 +--- .../timetodisplay.multiinstance.test.tsx | 403 ------------------ .../core/test/tracing/timetodisplay.test.tsx | 327 +++++++++++++- 6 files changed, 363 insertions(+), 601 deletions(-) delete mode 100644 packages/core/test/tracing/timetodisplay.multiinstance.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7082ea7c..eec634f0fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,14 +12,7 @@ - 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. - - Mounting a not-yet-ready peer on the next render commit (typical `useEffect` → `setState` → child mount waves) cancels any pending fire from an earlier-ready peer, preventing premature TTFD recording. Up-flips of the aggregate are deferred by one macrotask to absorb same-task peer mounts; down-flips are immediate. + - The existing `record` prop is unchanged BUT it is now deprecated in favor of `ready`. ### Fixes diff --git a/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts b/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts index e3ca401b3b..7f32cad1ef 100644 --- a/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts +++ b/packages/core/src/js/tracing/integrations/timeToDisplayIntegration.ts @@ -87,10 +87,6 @@ export const timeToDisplayIntegration = (): Integration => { event.timestamp = newTransactionEndTimestampSeconds; } - // Drop the per-span coordinator state now that we've read the native - // ttid/ttfd values. Prevents the module-level registries from - // accumulating entries for screens that outlive their span (keep-alive, - // idle-timeout discarded transactions, etc.). clearTimeToDisplayCoordinatorSpan(rootSpanId); return event; diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index 5d6253894a..7bdcc783e3 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -1,27 +1,6 @@ /** * Coordinator for multi-instance `` / `` * components on a single screen (active span). - * - * The aggregate "ready" state exposed via `isAllReady` is *deferred* on its - * way up: when the raw aggregate flips false→true, we schedule the public - * value to flip on the next macrotask. Down-flips (true→false) are - * immediate and cancel any pending up-flip. - * - * Why: in React 18, a typical "parent renders → parent useEffect setState - * → child mounts on next commit" wave executes synchronously inside one - * event-loop task. A `setTimeout(0)` reliably runs after that whole wave, - * so cross-commit-but-same-task peer registrations are absorbed before the - * coordinator declares itself ready. Without the defer, a header that - * registers and is alone-and-ready would emit `fullDisplay=true` - * immediately, the native reporter would fire on the next draw, and a - * sibling sidebar mounting on the next commit could only un-ready the - * aggregate after the (now stuck) native timestamp has already been - * recorded. - * - * The defer does NOT cover arbitrary-async deferred mounting (e.g. mount a - * checkpoint after a fetch resolves). That class of usage is documented - * against — the recommended pattern is to mount all checkpoints at screen - * mount with `ready=false` and flip them as data arrives. */ type Checkpoint = { ready: boolean }; @@ -30,36 +9,18 @@ type Listener = () => void; interface SpanRegistry { checkpoints: Map; listeners: Set; - /** - * Stable, deferred view of the aggregate exposed via `isAllReady`. Lags - * raw `computeAggregate` on up-flips by `READY_DEFER_MS`, immediate on - * down-flips. 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. - */ + // this value answers the question "are all checkpoints on this span ready?" + // when the raw value goes from false to true, aggregateReady does NOT flip immediately, it gets + // scheduled with setTimeout(0) in `reevaluate` function + // aggregateReady: boolean; - /** - * Pending up-flip timer. When non-null, an up-flip is scheduled but has - * not yet been applied to `aggregateReady`. Cleared either when the timer - * fires or when an intervening change cancels the pending up-flip. - */ + // when non-null, an up-flip is scheduled but has not yet been applied to `aggregateReady` pendingUpFlip: ReturnType | null; - /** - * Checkpoint ids whose components have unmounted but are kept in the - * registry to prevent a premature aggregate flip (sole-blocker safeguard). - * They participate in `computeAggregate` but are excluded from the - * "live work" count in `performCleanup`. - */ + // `sticky` is used indicate checkpints that gets cleared when the screen fully unmounts + // it's useful sticky: Set; } -/** - * Defer applied to up-flips. Zero macrotask is enough to absorb a same-task - * cascade of useEffect-driven child mounts in React 18. - */ -const READY_DEFER_MS = 0; - const TTID = 'ttid'; const TTFD = 'ttfd'; @@ -105,17 +66,7 @@ function computeAggregate(entry: SpanRegistry): boolean { return true; } -/** - * Recompute the raw aggregate and reconcile it with the cached - * `aggregateReady`. Up-flips are deferred to absorb same-task peer mounts; - * down-flips are immediate and cancel any pending up-flip. - * - * Transition matrix (raw, stable) → action: - * (false, false): no-op; cancel pending up-flip if any (it became stale). - * (true, true): no-op; cancel pending up-flip if any. - * (false, true): immediate down-flip + notify; cancel pending up-flip. - * (true, false): schedule up-flip if not already pending. - */ +// Recompute the raw aggregate and reconcile it with the cached `aggregateReady` function reevaluate(entry: SpanRegistry): void { const raw = computeAggregate(entry); @@ -131,10 +82,11 @@ function reevaluate(entry: SpanRegistry): void { return; } - // raw=true, stable=false: schedule deferred up-flip. if (entry.pendingUpFlip !== null) { return; } + // the delay here is set to 0 because in React 18 that + // will schedule the callback to be run asynchronously after the shortest possible delay entry.pendingUpFlip = setTimeout(() => { entry.pendingUpFlip = null; // Re-check on fire — a peer may have un-readied between schedule and now. @@ -143,7 +95,7 @@ function reevaluate(entry: SpanRegistry): void { } entry.aggregateReady = true; notifyListeners(entry); - }, READY_DEFER_MS); + }, 0); } function notifyListeners(entry: SpanRegistry): void { @@ -152,16 +104,6 @@ function notifyListeners(entry: SpanRegistry): void { } } -/** - * Delete the registry entry once there is no live work attached to it. - * - * "Live work" means either subscribed listeners (registry-mode components - * still mounted) or non-sticky checkpoints (still-mounted registrations). - * Sticky checkpoints (kept after a not-ready unmount; see `unregister`) - * exist only to prevent premature aggregate flips while the screen is still - * mounted; once every live counterpart is gone, they are orphaned and safe - * to drop along with the entry. - */ function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { const liveCheckpoints = entry.checkpoints.size - entry.sticky.size; if (liveCheckpoints === 0 && entry.listeners.size === 0) { @@ -170,22 +112,16 @@ function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegi } } -/** - * True iff removing `checkpointId` from `entry` would flip the aggregate - * from false to true — i.e., the checkpoint is the sole blocker. - * - * Used to detect the premature-fire scenario where a not-ready checkpoint - * unmounts while every other checkpoint is ready: deleting it would let the - * aggregate flip to true and immediately record TTFD/TTID, even though the - * unmounting source never actually became ready. - */ +// A bit of a hack but this is used to detect the premature-fire scenario +// where a not-ready checkpoint unmounts while every other checkpoint is ready: +// deleting it would let the aggregate flip to true and immediately record TTFD/TTID, +// even though the unmounting source never actually became ready. function isSoleBlocker(entry: SpanRegistry, checkpointId: string): boolean { if (entry.aggregateReady) { return false; } if (entry.checkpoints.size <= 1) { - // Removing the only checkpoint leaves the registry empty, which yields - // aggregate=false (per `computeAggregate`). No flip. + // because removing the only checkpoint leaves the registry empty return false; } const cp = entry.checkpoints.get(checkpointId); @@ -203,9 +139,6 @@ function isSoleBlocker(entry: SpanRegistry, checkpointId: string): boolean { return true; } -/** - * Register a checkpoint under (kind, parentSpanId). Returns an unregister fn. - */ export function registerCheckpoint( kind: DisplayKind, parentSpanId: string, @@ -221,10 +154,9 @@ export function registerCheckpoint( if (!e) { return; } - // If this checkpoint is the sole blocker, removing it would flip the - // aggregate to true and prematurely fire TTFD/TTID even though the - // unmounting source never became ready. Keep the checkpoint sticky; - // it gets cleared when the screen fully unmounts. + // if the checkpoint is the only blocker then removing it would flip the + // aggregate to true and fire TTFD/TTID even though the unmounting source never became ready. + // that's why we use `sticky` here to indicate that it gets cleared when the screen fully unmounts if (isSoleBlocker(e, checkpointId)) { e.sticky.add(checkpointId); performCleanup(kind, parentSpanId, e); @@ -238,9 +170,6 @@ export function registerCheckpoint( }; } -/** - * Update an existing checkpoint's ready state. - */ export function updateCheckpoint( kind: DisplayKind, parentSpanId: string, @@ -256,28 +185,19 @@ export function updateCheckpoint( reevaluate(entry); } -/** - * True if at least one checkpoint is registered AND all checkpoints are ready. - * Reads the cached aggregate — O(1). - */ +// Returns 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); return !!entry && entry.aggregateReady; } -/** - * Returns true if there is at least one registered checkpoint on this span. - */ +// 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 aggregate-ready transitions for a given span. The listener is - * called only when the aggregate flips, not on every individual checkpoint - * change. - */ +// Subscribe to aggregate-ready transitions for a given span export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Listener): () => void { const entry = getOrCreate(kind, parentSpanId); entry.listeners.add(listener); @@ -291,21 +211,10 @@ export function subscribe(kind: DisplayKind, parentSpanId: string, listener: Lis }; } -/** - * Drop coordinator state for `parentSpanId` across both kinds. - * - * Called by the time-to-display integration once a transaction has been - * processed, since the per-span coordinator state is no longer relevant - * after the native draw timestamps have been read. Without this hook, - * entries for screens that stay mounted past the end of their span - * (React Navigation keep-alive, idle-timeout discarded transactions, - * etc.) would accumulate in the module-level registries. - * - * Components that are still subscribed when their span is cleared remain - * functional: their next interaction recreates the entry under the same - * (now stale) parentSpanId. Since the integration has already read the - * native ttid/ttfd values for that span, any subsequent fires are inert. - */ +// Drop coordinator state for `parentSpanId` across both kinds. +// Called by the time-to-display integration once a transaction has been +// processed, since the per-span coordinator state is no longer relevant +// after the native draw timestamps have been read. export function clearSpan(parentSpanId: string): void { for (const kind of [TTID, TTFD] as const) { const entry = registries[kind].get(parentSpanId); @@ -316,9 +225,6 @@ export function clearSpan(parentSpanId: string): void { } } -/** - * Test-only. Clears all coordinator state. - */ export function _resetTimeToDisplayCoordinator(): void { for (const kind of [TTID, TTFD] as const) { for (const entry of registries[kind].values()) { diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index 61153199e4..264185b961 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -61,29 +61,16 @@ 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. - */ + /** @deprecated Use `ready` instead. `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`. - * - * - * - */ + // Marks this checkpoint as ready. ready?: boolean; }; /** * Component to measure time to initial display. * - * Single instance: - * - * - * Multiple instances coordinating on one screen: + * Usage example: * * */ @@ -106,10 +93,7 @@ export function TimeToInitialDisplay(props: TimeToDisplayProps): React.ReactElem /** * Component to measure time to full display. * - * Single instance: - * - * - * Multiple instances coordinating on one screen: + * Usage example: * * */ @@ -125,28 +109,6 @@ export function TimeToFullDisplay(props: TimeToDisplayProps): React.ReactElement ); } -/** - * 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; function useCoordinatedDisplay( @@ -154,7 +116,6 @@ function useCoordinatedDisplay( parentSpanId: string | undefined, props: TimeToDisplayProps, ): boolean { - // Stable per-instance id. `useRef` is available since React 16.8. const checkpointIdRef = useRef(null); if (checkpointIdRef.current === null) { checkpointIdRef.current = `cp-${nextCheckpointId++}`; @@ -162,10 +123,11 @@ function useCoordinatedDisplay( const checkpointId = checkpointIdRef.current; const [, force] = useReducer((x: number) => x + 1, 0); + // useRegistry means we might use multiple TTID/TTFD components const useRegistry = props.ready !== undefined; const localReady = useRegistry ? !!props.ready : !!props.record; - // Emit deprecation / conflict warnings once per component instance. + // We do it this way because we don't want warnings being thrown on every re-render const warnedRef = useRef(false); useEffect(() => { if (!__DEV__ || warnedRef.current) return; @@ -179,9 +141,6 @@ function useCoordinatedDisplay( // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - // Subscribe FIRST so this component receives its own registration notify - // (and any peer notifications) on mount. Only registry-mode components - // need peer notifications. useEffect(() => { if (!parentSpanId || !useRegistry) { return undefined; @@ -189,16 +148,10 @@ function useCoordinatedDisplay( return subscribe(kind, parentSpanId, force); }, [kind, parentSpanId, useRegistry]); - // Tracks whether this component's checkpoint is currently registered with - // the coordinator. Used to detect the pre-register render so we don't - // inherit a peer's ready=true verdict before our own checkpoint exists in - // the aggregate. Reset on unregister so re-mount under a new active span - // re-clamps correctly. + // Tracks if this component's checkpoint is currently registered with the coordinator const registeredRef = useRef(false); - // 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. + // Register on mount / when the active span changes useEffect(() => { if (!parentSpanId || !useRegistry) { return undefined; @@ -212,8 +165,7 @@ function useCoordinatedDisplay( // eslint-disable-next-line react-hooks/exhaustive-deps }, [kind, parentSpanId, useRegistry, checkpointId]); - // Propagate ready transitions to the registry. Legacy-mode components - // skip this — they propagate their value directly via the returned boolean. + // Propagate ready transitions to the registry useEffect(() => { if (!parentSpanId || !useRegistry) { return; @@ -221,23 +173,16 @@ function useCoordinatedDisplay( updateCheckpoint(kind, parentSpanId, checkpointId, localReady); }, [kind, parentSpanId, useRegistry, checkpointId, localReady]); + // Main logic for "readiness" 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, pre-register render: our checkpoint is not yet in the - // registry, so `isAllReady` reflects only peer state. Combine it with our - // own `localReady` so we never inherit a peer's true verdict before our - // own checkpoint has had a chance to report. if (!registeredRef.current) { return localReady && isAllReady(kind, parentSpanId); } - // Registry, post-register: gated on the per-span aggregate, which now - // includes our checkpoint. return isAllReady(kind, parentSpanId); } diff --git a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx b/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx deleted file mode 100644 index d4b5423016..0000000000 --- a/packages/core/test/tracing/timetodisplay.multiinstance.test.tsx +++ /dev/null @@ -1,403 +0,0 @@ -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; - -/** - * Flush the coordinator's deferred up-flip timer + any consequent React - * re-renders. Wrapped in act() so React applies the resulting state updates. - */ -function flushReadyDefer(): void { - act(() => { - jest.runOnlyPendingTimers(); - }); -} - -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(); - flushReadyDefer(); - 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( - <> - - - , - ); - flushReadyDefer(); - 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(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - 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(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(true); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - activeSpan?.end(); - }); - }); - - test('unmounting the sole blocker does NOT emit ready (sticky safeguard)', () => { - // A conditionally-rendered loading section that disappears before its - // data resolves must not trick TTFD into firing as if the screen were - // fully displayed. The sole-blocker checkpoint is kept sticky in the - // registry, so its unmount cannot flip the aggregate to true. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = ({ showBlocker }: { showBlocker: boolean }) => ( - <> - - {showBlocker ? : null} - - ); - - const tree = render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(false); - - activeSpan?.end(); - }); - }); - - test('unmounting a non-sole-blocker resolves the aggregate when remaining peers are ready', () => { - // The sticky safeguard only applies to *sole* blockers. If other - // not-ready peers exist, an unmount can't flip the aggregate to true, - // so it removes normally. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = ({ aReady, showB }: { aReady: boolean; showB: boolean }) => ( - <> - - {showB ? : null} - - ); - - const tree = render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - // Unmount B while A is also not-ready: not a sole-blocker case, B - // removes normally; aggregate still blocked by A. - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(false); - - // Now flip A to ready: aggregate flips, A's reporter emits. - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(true); - - activeSpan?.end(); - }); - }); - - 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; - - const Screen = ({ rec, rdy }: { rec: boolean; rdy: boolean }) => ( - <> - - - - ); - - // record=true fires independently; ready=false blocks the ready reporter. - // The tail reflects: [record:true, ready:false] → fullDisplay=true present. - const tree = render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - // record=false stops emitting; ready=true now fires. - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - // Both fire. - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - activeSpan?.end(); - }); - }); - - 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( - <> - - - , - ); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - activeSpan?.end(); - }); - }); - - test('late-mounting `ready={false}` peer does not inherit existing peer’s ready=true (pre-register clamp)', () => { - // Bug class: a fresh component on its first render calls `isAllReady` - // before its useEffect has registered its checkpoint. If a peer was - // already registered ready, the new instance would see aggregate=true - // and emit `fullDisplay=true` from its own reporter on its very first - // render — a premature fire whose timestamp would overwrite the (also - // premature) earlier one via native last-wins. - // - // The clamp `localReady && isAllReady` guarantees: if the new - // instance's local prop is false, it can never emit true on its first - // render even when peers are already ready. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = ({ showLate }: { showLate: boolean }) => ( - <> - - {showLate ? : null} - - ); - - const tree = render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(true); - - // Mount the late ready=false peer. Its very first render must NOT emit - // true even though aggregate is currently true (peer A is ready). - act(() => tree.rerender()); - // After both renders settle, neither reporter should have its current - // prop set to true. - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - activeSpan?.end(); - }); - }); - - test('same-task wave: header alone-and-ready does not fire when sibling mounts on next commit', () => { - // Reviewer's scenario: a header that loads instantly would otherwise - // finalize TTFD before a sibling that mounts a tick later registers as - // not-ready. The coordinator's deferred up-flip catches this race as - // long as the late mount happens within the same event-loop task - // (the typical "parent useEffect setState → child mount" wave). - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = (): React.ReactElement => { - const [showSidebar, setShowSidebar] = React.useState(false); - React.useEffect(() => { - setShowSidebar(true); - }, []); - return ( - <> - - {showSidebar ? : null} - - ); - }; - - render(); - // Both the synchronous header mount and the deferred-via-useEffect - // sidebar mount have completed; flushing the up-flip timer must NOT - // produce a fullDisplay=true emission, because the sidebar is now a - // registered blocker. - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - 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. - flushReadyDefer(); - 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(); - flushReadyDefer(); - 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(); - flushReadyDefer(); - expect(tailHasInitialDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasInitialDisplay(spanId, 2)).toBe(true); - - activeSpan?.end(); - }); - }); -}); diff --git a/packages/core/test/tracing/timetodisplay.test.tsx b/packages/core/test/tracing/timetodisplay.test.tsx index 2542fff3f8..750d70c24d 100644 --- a/packages/core/test/tracing/timetodisplay.test.tsx +++ b/packages/core/test/tracing/timetodisplay.test.tsx @@ -20,10 +20,11 @@ import * as mockedtimetodisplaynative from './mockedtimetodisplaynative'; jest.mock('../../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative); -import { render } from '@testing-library/react-native'; +import { act, render } from '@testing-library/react-native'; import * as React from 'react'; import { timeToDisplayIntegration } from '../../src/js/tracing/integrations/timeToDisplayIntegration'; +import { _resetTimeToDisplayCoordinator } from '../../src/js/tracing/timeToDisplayCoordinator'; import { SPAN_ORIGIN_MANUAL_UI_TIME_TO_DISPLAY } from '../../src/js/tracing/origin'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, @@ -48,6 +49,28 @@ jest.mock('../../src/js/utils/environment', () => ({ const { mockRecordedTimeToDisplay, getMockedOnDrawReportedProps, clearMockedOnDrawReportedProps } = mockedtimetodisplaynative; +/** Flush the coordinator's deferred up-flip + any consequent React re-renders. */ +function flushReadyDefer(): void { + act(() => { + jest.runOnlyPendingTimers(); + }); +} + +/** + * The mock records every render of every native draw reporter. We slice the + * tail of the prop log to inspect the converged post-effect state of all + * currently-mounted reporters for a given span. + */ +function tailHasFullDisplay(parentSpanId: string, mountedReporterCount: number): boolean { + const props = getMockedOnDrawReportedProps().filter(p => p.parentSpanId === parentSpanId); + return props.slice(-mountedReporterCount).some(p => p.fullDisplay === true); +} + +function tailHasInitialDisplay(parentSpanId: string, mountedReporterCount: number): boolean { + const props = getMockedOnDrawReportedProps().filter(p => p.parentSpanId === parentSpanId); + return props.slice(-mountedReporterCount).some(p => p.initialDisplay === true); +} + jest.useFakeTimers({ advanceTimers: true, doNotFake: ['performance'], // Keep real performance API @@ -58,6 +81,7 @@ describe('TimeToDisplay', () => { beforeEach(() => { clearMockedOnDrawReportedProps(); + _resetTimeToDisplayCoordinator(); getCurrentScope().clear(); getIsolationScope().clear(); getGlobalScope().clear(); @@ -744,4 +768,305 @@ describe('Frame Data', () => { expect(ttidSpan).toBeDefined(); expect(ttidSpan!.data).not.toHaveProperty('frames.delay'); }); + + describe('multi-instance (`ready` prop)', () => { + test('legacy: single `record` instance behaves identically to today', () => { + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render(); + flushReadyDefer(); + 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( + <> + + + , + ); + flushReadyDefer(); + 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(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + 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(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('unmounting the sole blocker does NOT emit ready (sticky safeguard)', () => { + // A conditionally-rendered loading section that disappears before its + // data resolves must not trick TTFD into firing as if the screen were + // fully displayed. The sole-blocker checkpoint is kept sticky in the + // registry, so its unmount cannot flip the aggregate to true. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ showBlocker }: { showBlocker: boolean }) => ( + <> + + {showBlocker ? : null} + + ); + + const tree = render(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(false); + + activeSpan?.end(); + }); + }); + + test('unmounting a non-sole-blocker resolves the aggregate when remaining peers are ready', () => { + // The sticky safeguard only applies to *sole* blockers. If other + // not-ready peers exist, an unmount can't flip the aggregate to true, + // so it removes normally. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ aReady, showB }: { aReady: boolean; showB: boolean }) => ( + <> + + {showB ? : null} + + ); + + const tree = render(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + activeSpan?.end(); + }); + }); + + 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. + 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(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + + test('legacy: bare does not block `ready` peers', () => { + // Backward compat for layout-placeholder usage. A bare component is a no-op. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + render( + <> + + + , + ); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + activeSpan?.end(); + }); + }); + + test('legacy: two `record` peers fire independently (no coordination)', () => { + // 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( + <> + + + , + ); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(true); + activeSpan?.end(); + }); + }); + + test('late-mounting `ready={false}` peer does not inherit existing peer\u2019s ready=true (pre-register clamp)', () => { + // A fresh component on its first render calls isAllReady before its + // useEffect has registered. The clamp `localReady && isAllReady` ensures + // it can never inherit a peer's true verdict on its first render. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = ({ showLate }: { showLate: boolean }) => ( + <> + + {showLate ? : null} + + ); + + const tree = render(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 1)).toBe(true); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + activeSpan?.end(); + }); + }); + + test('same-task wave: header alone-and-ready does not fire when sibling mounts on next commit', () => { + // The defining race the deferred up-flip protects against: header + // registers ready=true alone, sibling mounts a tick later via the typical + // parent-useEffect-setState wave. setTimeout(0) defers past the entire + // task so the late mount cancels the pending up-flip. + startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { + const spanId = spanToJSON(activeSpan!).span_id; + + const Screen = (): React.ReactElement => { + const [showSidebar, setShowSidebar] = React.useState(false); + React.useEffect(() => { + setShowSidebar(true); + }, []); + return ( + <> + + {showSidebar ? : null} + + ); + }; + + render(); + flushReadyDefer(); + expect(tailHasFullDisplay(spanId, 2)).toBe(false); + + 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(); + flushReadyDefer(); + 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(); + flushReadyDefer(); + expect(tailHasInitialDisplay(spanId, 2)).toBe(false); + + act(() => tree.rerender()); + flushReadyDefer(); + expect(tailHasInitialDisplay(spanId, 2)).toBe(true); + + activeSpan?.end(); + }); + }); + }); }); From a54077b931e8bc1b1088f2537ceef8b6b6f737f6 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 12:56:56 +0200 Subject: [PATCH 11/14] Fixes to tests --- .../core/src/js/tracing/timetodisplay.tsx | 7 +- .../tracing/timeToDisplayCoordinator.test.ts | 138 +-------------- .../core/test/tracing/timetodisplay.test.tsx | 159 +----------------- 3 files changed, 16 insertions(+), 288 deletions(-) diff --git a/packages/core/src/js/tracing/timetodisplay.tsx b/packages/core/src/js/tracing/timetodisplay.tsx index 264185b961..1814e24a9e 100644 --- a/packages/core/src/js/tracing/timetodisplay.tsx +++ b/packages/core/src/js/tracing/timetodisplay.tsx @@ -521,7 +521,12 @@ function createTimeToDisplay({ }; }); - return ; + // gate both legacy `record` and the new `ready` checkpoint on focus + // + // the idea here is that wrappers built via createTimeToFullDisplay/createTimeToInitialDisplay + // can only record TTID/TTFD on a focused screen + const gatedReady = props.ready === undefined ? undefined : focused && props.ready; + return ; }; TimeToDisplayWrapper.displayName = 'TimeToDisplayWrapper'; diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index d4e614daaa..66743fbcb0 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -11,7 +11,6 @@ import { const SPAN_FIRST = 'span-first'; const SPAN_SECOND = 'span-second'; -/** Flush the coordinator's deferred up-flip timer. */ function flushDefer(): void { jest.runOnlyPendingTimers(); } @@ -53,7 +52,7 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); - test('late-registering not-ready checkpoint un-readies the aggregate', () => { + test('late-registering not-ready checkpoint makes the aggregate "non-ready"', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); flushDefer(); @@ -63,58 +62,6 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); }); - test('unregistering the sole blocker keeps the aggregate not-ready (sticky)', () => { - // A not-ready checkpoint that unmounts while it is the sole blocker is - // kept in the registry to prevent a premature aggregate flip. Otherwise - // a conditionally-rendered loading section that disappears before its - // data resolves would silently record an incomplete display. - 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(false); - // The sticky 'b' is still tracked so a subsequent component on the same - // span continues to see it as a blocker. - expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(true); - }); - - test('unregistering a non-sole-blocker not-ready checkpoint removes it normally', () => { - // When other not-ready checkpoints are still present, removing one - // would not flip the aggregate, so the sticky safeguard does not apply. - registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); - const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); - - unregisterB(); - expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); - // 'a' remains, 'b' is gone. - updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); - flushDefer(); - expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); - }); - - test('unregistering a ready checkpoint never goes sticky', () => { - registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); - const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); - - unregisterB(); - // 'a' is still blocking; 'b' was ready so removing it is safe. - expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); - updateCheckpoint('ttfd', SPAN_FIRST, 'a', true); - flushDefer(); - expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); - }); - - test('unregistering the last checkpoint leaves aggregate not-ready', () => { - const unregister = registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); - flushDefer(); - 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); @@ -123,7 +70,7 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_SECOND)).toBe(false); }); - test('different kinds are independent', () => { + test('different kinds of spans are independent', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); registerCheckpoint('ttid', SPAN_FIRST, 'a', false); flushDefer(); @@ -131,21 +78,6 @@ describe('timeToDisplayCoordinator', () => { 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 only on aggregate-ready flips', () => { const listener = jest.fn(); subscribe('ttfd', SPAN_FIRST, listener); @@ -159,25 +91,6 @@ describe('timeToDisplayCoordinator', () => { expect(listener).toHaveBeenCalledTimes(2); }); - 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); - flushDefer(); - expect(listener).toHaveBeenCalledTimes(1); - }); - test('unsubscribe stops further notifications', () => { const listener = jest.fn(); const unsubscribe = subscribe('ttfd', SPAN_FIRST, listener); @@ -193,68 +106,25 @@ describe('timeToDisplayCoordinator', () => { expect(listener).not.toHaveBeenCalled(); }); - test('up-flip is deferred so a same-task late-mounting peer can cancel it', () => { - // The defining race: header registers ready=true alone, sidebar mounts - // a tick later (e.g. parent useEffect setState→child mount). Without the - // defer, header would fire instantly. With the defer, sidebar's - // registration cancels the pending up-flip before it fires. + test('up-flip is deferred', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'header', true); - // Aggregate is *raw* ready, but `isAllReady` (deferred view) is still false. expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); - // Same-task: sidebar mounts before the timer macrotask runs. registerCheckpoint('ttfd', SPAN_FIRST, 'sidebar', false); flushDefer(); - // Aggregate must NOT have flipped — the defer protected us. expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); - - // Sidebar resolves — now we get a real ready transition. updateCheckpoint('ttfd', SPAN_FIRST, 'sidebar', true); flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); }); - test('down-flip is immediate (cancels pending up-flip and not deferred itself)', () => { + test('down-flip is immediate', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); - // No flushDefer: down-flip is immediate. expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); }); - - test('clearSpan drops all coordinator state for a span', () => { - // Simulates the integration calling clearSpan after popTimeToDisplayFor - // returns. Prevents the registries from accumulating entries for screens - // that outlive their span (keep-alive, idle-timeout discarded txns, etc.). - registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); - registerCheckpoint('ttid', SPAN_FIRST, 'a', true); - registerCheckpoint('ttfd', SPAN_SECOND, 'a', true); - flushDefer(); - expect(isAllReady('ttfd', SPAN_FIRST)).toBe(true); - flushDefer(); - expect(isAllReady('ttid', SPAN_FIRST)).toBe(true); - flushDefer(); - expect(isAllReady('ttfd', SPAN_SECOND)).toBe(true); - - clearSpan(SPAN_FIRST); - - expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); - expect(hasAnyCheckpoints('ttid', SPAN_FIRST)).toBe(false); - // Other spans untouched. - flushDefer(); - expect(isAllReady('ttfd', SPAN_SECOND)).toBe(true); - }); - - test('clearSpan also drops sticky checkpoints', () => { - registerCheckpoint('ttfd', SPAN_FIRST, 'a', true); - const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); - unregisterB(); // becomes sticky - expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(true); - - clearSpan(SPAN_FIRST); - expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(false); - }); }); diff --git a/packages/core/test/tracing/timetodisplay.test.tsx b/packages/core/test/tracing/timetodisplay.test.tsx index 750d70c24d..3c59f86afa 100644 --- a/packages/core/test/tracing/timetodisplay.test.tsx +++ b/packages/core/test/tracing/timetodisplay.test.tsx @@ -769,8 +769,8 @@ describe('Frame Data', () => { expect(ttidSpan!.data).not.toHaveProperty('frames.delay'); }); - describe('multi-instance (`ready` prop)', () => { - test('legacy: single `record` instance behaves identically to today', () => { + describe('multi-instance', () => { + test('legacy: single instance behaves identically to today', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; render(); @@ -780,7 +780,7 @@ describe('Frame Data', () => { }); }); - test('two `ready={false}` instances do not emit', () => { + test('two ready=false instances do not emit', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; render( @@ -795,7 +795,7 @@ describe('Frame Data', () => { }); }); - test('two `ready` instances emit only when both are ready', () => { + test('two ready instances emit only when both are ready', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -822,7 +822,7 @@ describe('Frame Data', () => { }); }); - test('late-mounting `ready={false}` un-readies an already-ready aggregate', () => { + test('late-mounting makes a previously ready aggregate non-ready', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -849,11 +849,7 @@ describe('Frame Data', () => { }); }); - test('unmounting the sole blocker does NOT emit ready (sticky safeguard)', () => { - // A conditionally-rendered loading section that disappears before its - // data resolves must not trick TTFD into firing as if the screen were - // fully displayed. The sole-blocker checkpoint is kept sticky in the - // registry, so its unmount cannot flip the aggregate to true. + test('unmounting the sole blocker does NOT emit ready', () => { startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -877,9 +873,6 @@ describe('Frame Data', () => { }); test('unmounting a non-sole-blocker resolves the aggregate when remaining peers are ready', () => { - // The sticky safeguard only applies to *sole* blockers. If other - // not-ready peers exist, an unmount can't flip the aggregate to true, - // so it removes normally. startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { const spanId = spanToJSON(activeSpan!).span_id; @@ -906,123 +899,6 @@ describe('Frame Data', () => { }); }); - 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. - 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(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - - activeSpan?.end(); - }); - }); - - test('legacy: bare does not block `ready` peers', () => { - // Backward compat for layout-placeholder usage. A bare component is a no-op. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - render( - <> - - - , - ); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - activeSpan?.end(); - }); - }); - - test('legacy: two `record` peers fire independently (no coordination)', () => { - // 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( - <> - - - , - ); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(true); - activeSpan?.end(); - }); - }); - - test('late-mounting `ready={false}` peer does not inherit existing peer\u2019s ready=true (pre-register clamp)', () => { - // A fresh component on its first render calls isAllReady before its - // useEffect has registered. The clamp `localReady && isAllReady` ensures - // it can never inherit a peer's true verdict on its first render. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = ({ showLate }: { showLate: boolean }) => ( - <> - - {showLate ? : null} - - ); - - const tree = render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 1)).toBe(true); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - activeSpan?.end(); - }); - }); - - test('same-task wave: header alone-and-ready does not fire when sibling mounts on next commit', () => { - // The defining race the deferred up-flip protects against: header - // registers ready=true alone, sibling mounts a tick later via the typical - // parent-useEffect-setState wave. setTimeout(0) defers past the entire - // task so the late mount cancels the pending up-flip. - startSpanManual({ name: 'Screen', startTime: secondAgoTimestampMs() }, (activeSpan: Span | undefined) => { - const spanId = spanToJSON(activeSpan!).span_id; - - const Screen = (): React.ReactElement => { - const [showSidebar, setShowSidebar] = React.useState(false); - React.useEffect(() => { - setShowSidebar(true); - }, []); - return ( - <> - - {showSidebar ? : null} - - ); - }; - - render(); - flushReadyDefer(); - expect(tailHasFullDisplay(spanId, 2)).toBe(false); - - activeSpan?.end(); - }); - }); - test('different active spans have independent registries', () => { let firstSpanId = ''; let secondSpanId = ''; @@ -1045,28 +921,5 @@ describe('Frame Data', () => { 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(); - flushReadyDefer(); - expect(tailHasInitialDisplay(spanId, 2)).toBe(false); - - act(() => tree.rerender()); - flushReadyDefer(); - expect(tailHasInitialDisplay(spanId, 2)).toBe(true); - - activeSpan?.end(); - }); - }); }); }); From 57d1b6799d443d22b85737bb025b399b6b8a3d23 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 15:10:19 +0200 Subject: [PATCH 12/14] Changelog fix --- CHANGELOG.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f172714f6..3b8022cbd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,7 @@ > make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first. -## 8.11.0 - -### Features - -- Use `accessibilityLabel`, `aria-label`, and `testID` as fallback labels for touch breadcrumbs when `sentry-label` is not set ([#6103](https://github.com/getsentry/sentry-react-native/pull/6103)) +## Unreleased ### Features @@ -18,6 +14,12 @@ - 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`. - The existing `record` prop is unchanged BUT it is now deprecated in favor of `ready`. +## 8.11.0 + +### Features + +- Use `accessibilityLabel`, `aria-label`, and `testID` as fallback labels for touch breadcrumbs when `sentry-label` is not set ([#6103](https://github.com/getsentry/sentry-react-native/pull/6103)) + ### Fixes - Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076)) From f2744988ae9002fb2e92e417c8c7cea333aa7fd1 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 15:14:16 +0200 Subject: [PATCH 13/14] Fix --- .../core/src/js/tracing/timeToDisplayCoordinator.ts | 6 ++++-- .../test/tracing/timeToDisplayCoordinator.test.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index 7bdcc783e3..b9194ae99b 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -105,8 +105,10 @@ function notifyListeners(entry: SpanRegistry): void { } function performCleanup(kind: DisplayKind, parentSpanId: string, entry: SpanRegistry): void { - const liveCheckpoints = entry.checkpoints.size - entry.sticky.size; - if (liveCheckpoints === 0 && entry.listeners.size === 0) { + if (entry.sticky.size > 0) { + return; + } + if (entry.checkpoints.size === 0 && entry.listeners.size === 0) { cancelPendingUpFlip(entry); registries[kind].delete(parentSpanId); } diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index 66743fbcb0..9526f1ef3d 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -127,4 +127,16 @@ describe('timeToDisplayCoordinator', () => { registerCheckpoint('ttfd', SPAN_FIRST, 'b', false); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); }); + + test('sticky-blocker survives sibling unmounts and continues to block new peers', () => { + const unregisterA = registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); + const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); + + unregisterA(); // A becomes sticky + unregisterB(); + + registerCheckpoint('ttfd', SPAN_FIRST, 'c', true); + flushDefer(); + expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + }); }); From 1582d1d2dfcdf0e88532836234bcd9b393c4e395 Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Thu, 7 May 2026 15:57:53 +0200 Subject: [PATCH 14/14] Fix --- .../core/src/js/tracing/timeToDisplayCoordinator.ts | 11 +++++++++++ .../test/tracing/timeToDisplayCoordinator.test.ts | 12 +++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts index b9194ae99b..da638834b8 100644 --- a/packages/core/src/js/tracing/timeToDisplayCoordinator.ts +++ b/packages/core/src/js/tracing/timeToDisplayCoordinator.ts @@ -148,6 +148,17 @@ export function registerCheckpoint( ready: boolean, ): () => void { const entry = getOrCreate(kind, parentSpanId); + + // Any new registration means the screen's component graph is changing. + // Drop leftover sticky entries from previous unmount cycles -- otherwise + // a remounted checkpoint would be permanently blockedю + if (entry.sticky.size > 0) { + for (const id of entry.sticky) { + entry.checkpoints.delete(id); + } + entry.sticky.clear(); + } + entry.checkpoints.set(checkpointId, { ready }); reevaluate(entry); diff --git a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts index 9526f1ef3d..76b11dfb66 100644 --- a/packages/core/test/tracing/timeToDisplayCoordinator.test.ts +++ b/packages/core/test/tracing/timeToDisplayCoordinator.test.ts @@ -128,15 +128,13 @@ describe('timeToDisplayCoordinator', () => { expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); }); - test('sticky-blocker survives sibling unmounts and continues to block new peers', () => { - const unregisterA = registerCheckpoint('ttfd', SPAN_FIRST, 'a', false); - const unregisterB = registerCheckpoint('ttfd', SPAN_FIRST, 'b', true); - - unregisterA(); // A becomes sticky - unregisterB(); + test('sole-blocker unmount with no remount keeps the aggregate blocked', () => { + registerCheckpoint('ttfd', SPAN_FIRST, 'header', true); + const unregisterLoader = registerCheckpoint('ttfd', SPAN_FIRST, 'loader', false); - registerCheckpoint('ttfd', SPAN_FIRST, 'c', true); + unregisterLoader(); // sticky flushDefer(); expect(isAllReady('ttfd', SPAN_FIRST)).toBe(false); + expect(hasAnyCheckpoints('ttfd', SPAN_FIRST)).toBe(true); }); });