From 016ea7b7a917577f58485bfb19d041d746f204b2 Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Thu, 13 Aug 2026 16:19:30 +0700 Subject: [PATCH] fix: stop rebuilding the web measurement probe on every render The measurement effect in NativeSafeAreaProvider.web listed onInsetsChange in its dependency array. SafeAreaListener passes a fresh inline arrow on every render, so every render of a SafeAreaListener tore down and rebuilt the hidden probe element, its transitionend and resize listeners and the ResizeObserver, and re-measured. The effect never needs the callback's identity, only the latest callback, so read it through a ref and drop it from the dependencies. Setup now runs once per mount while inset and frame changes still propagate to the most recent callback. --- src/NativeSafeAreaProvider.web.tsx | 12 +- .../NativeSafeAreaProvider.web-test.tsx | 123 ++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/src/NativeSafeAreaProvider.web.tsx b/src/NativeSafeAreaProvider.web.tsx index 8282869..f96d4a3 100644 --- a/src/NativeSafeAreaProvider.web.tsx +++ b/src/NativeSafeAreaProvider.web.tsx @@ -19,6 +19,14 @@ export function NativeSafeAreaProvider({ }: NativeSafeAreaProviderProps) { const viewRef = React.useRef(null); + // Callers commonly pass an inline callback, so its identity changes on every + // render. Read it through a ref to keep it out of the effect's dependencies: + // the measurement setup below only ever needs to call the latest one. + const onInsetsChangeRef = React.useRef(onInsetsChange); + React.useEffect(() => { + onInsetsChangeRef.current = onInsetsChange; + }, [onInsetsChange]); + React.useEffect(() => { // Skip for SSR. if (typeof document === 'undefined') { @@ -81,7 +89,7 @@ export function NativeSafeAreaProvider({ } // @ts-ignore: missing properties - onInsetsChange({ nativeEvent: { insets, frame } }); + onInsetsChangeRef.current({ nativeEvent: { insets, frame } }); }; element.addEventListener(getSupportedTransitionEvent(), onEnd); window.addEventListener('resize', onEnd); @@ -105,7 +113,7 @@ export function NativeSafeAreaProvider({ resizeObserver?.disconnect(); element.remove(); }; - }, [onInsetsChange]); + }, []); return ( diff --git a/src/__tests__/NativeSafeAreaProvider.web-test.tsx b/src/__tests__/NativeSafeAreaProvider.web-test.tsx index c34aaf4..5fbf5c6 100644 --- a/src/__tests__/NativeSafeAreaProvider.web-test.tsx +++ b/src/__tests__/NativeSafeAreaProvider.web-test.tsx @@ -19,6 +19,7 @@ import type { Metrics, } from '../SafeArea.types'; import { NativeSafeAreaProvider } from '../NativeSafeAreaProvider.web'; +import { SafeAreaListener } from '../SafeAreaContext'; jest.mock('react-native', () => { const ReactActual = jest.requireActual('react'); @@ -29,9 +30,18 @@ jest.mock('react-native', () => { >(function View({ children }, ref) { return ReactActual.createElement('div', { ref }, children); }), + StyleSheet: { create: (styles: T): T => styles }, + Dimensions: { get: () => ({ width: WINDOW_WIDTH, height: WINDOW_HEIGHT }) }, }; }); +// `SafeAreaContext` imports the platform-agnostic `./NativeSafeAreaProvider`, +// which the react-native jest preset resolves to the native implementation. +// Point it at the web one so this suite exercises the web code path. +jest.mock('../NativeSafeAreaProvider', () => + jest.requireActual('../NativeSafeAreaProvider.web'), +); + ( globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean } ).IS_REACT_ACT_ENVIRONMENT = true; @@ -119,6 +129,28 @@ function spyOnBoundingClientRect() { return jest.spyOn(Element.prototype, 'getBoundingClientRect'); } +/** + * Counts how many times the hidden measurement probe is attached to the + * document. The probe is torn down and re-attached whenever the measurement + * effect re-runs, so at any instant only one is present: the churn is only + * visible in the number of attachments. + */ +function trackProbeAttachments(): { count: number } { + const counter = { count: 0 }; + const appendChild = document.body.appendChild.bind(document.body); + jest + .spyOn(document.body, 'appendChild') + .mockImplementation((node: T): T => { + if ( + (node as Node as HTMLElement).style?.transitionProperty === 'padding' + ) { + counter.count += 1; + } + return appendChild(node); + }); + return counter; +} + let root: Root | null = null; let host: HTMLElement | null = null; let rectMock: ReturnType; @@ -281,4 +313,95 @@ describe('NativeSafeAreaProvider.web', () => { frame: { x: 0, y: 0, width: 800, height: 600 }, }); }); + + function mountRoot(): Root { + const newHost = document.createElement('div'); + document.body.appendChild(newHost); + const newRoot = createRoot(newHost); + host = newHost; + root = newRoot; + return newRoot; + } + + it('does not rebuild the measurement probe when the provider re-renders', () => { + const onInsetsChange = jest.fn(); + const newRoot = mountRoot(); + const probes = trackProbeAttachments(); + + // A fresh callback identity on every render, which is what an inline + // arrow in the parent produces. + const render = () => + act(() => { + newRoot.render( + onInsetsChange(e)} />, + ); + }); + + render(); + render(); + render(); + + expect(probes.count).toBe(1); + expect(ResizeObserverMock.instances).toHaveLength(1); + expect(onInsetsChange).toHaveBeenCalledTimes(1); + }); + + it('does not rebuild the measurement probe when a SafeAreaListener re-renders', () => { + const onChange = jest.fn(); + const newRoot = mountRoot(); + const probes = trackProbeAttachments(); + + // `onChange` is the same function on every render, so any churn comes + // from `SafeAreaListener` itself and not from the caller. + const render = () => + act(() => { + newRoot.render(); + }); + + render(); + render(); + render(); + + expect(probes.count).toBe(1); + expect(ResizeObserverMock.instances).toHaveLength(1); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('reports later changes to the most recent onInsetsChange', () => { + rectMock.mockReturnValue(makeRect(0, 0, WINDOW_WIDTH, WINDOW_HEIGHT)); + const first = jest.fn(); + const second = jest.fn(); + const newRoot = mountRoot(); + + const render = (onInsetsChange: InsetChangeNativeCallback) => + act(() => { + newRoot.render( + , + ); + }); + + render(first); + expect(first).toHaveBeenCalledTimes(1); + + // Swapping the callback must not re-measure on its own. + render(second); + expect(second).not.toHaveBeenCalled(); + + // A genuine change must still propagate, and to the current callback. + rectMock.mockReturnValue(makeRect(0, 100, WINDOW_WIDTH, 668)); + triggerResizeObservers(); + + expect(second).toHaveBeenCalledTimes(1); + expect(lastMetrics(second)).toEqual({ + insets: { + top: 0, + bottom: WINDOW_INSETS.bottom, + left: WINDOW_INSETS.left, + right: WINDOW_INSETS.right, + }, + frame: { x: 0, y: 100, width: WINDOW_WIDTH, height: 668 }, + }); + // The stale callback is not called again. + expect(first).toHaveBeenCalledTimes(1); + }); });