Skip to content

fix: stop rebuilding the web measurement probe on every render - #748

Open
giaBaoJS wants to merge 1 commit into
appandflow:mainfrom
giaBaoJS:fix/web-remeasure-on-every-render
Open

giaBaoJS wants to merge 1 commit into
appandflow:mainfrom
giaBaoJS:fix/web-remeasure-on-every-render

Conversation

@giaBaoJS

@giaBaoJS giaBaoJS commented Aug 13, 2026

Copy link
Copy Markdown

There is no issue filed for this - I found it while reading NativeSafeAreaProvider.web.tsx, so I'm leading with the reproduction rather than a report.

Summary

What happens

The measurement effect in src/NativeSafeAreaProvider.web.tsx had [onInsetsChange] in its dependency array. SafeAreaListener (src/SafeAreaContext.tsx:137) builds its onInsetsChange as an inline arrow inside its own render:

onInsetsChange={(e) => {
  onChange({ insets: e.nativeEvent.insets, frame: e.nativeEvent.frame });
}}

That arrow is a new function on every render of SafeAreaListener, so the effect's cleanup and setup ran on every render of the listener. Each cycle:

  • removes the hidden probe <div> from document.body and appends a freshly created one,
  • removes and re-adds the transitionend listener on the probe and the resize listener on window,
  • disconnects the ResizeObserver and constructs and re-observes a new one,
  • calls window.getComputedStyle(element) and providerElement.getBoundingClientRect() and fires onInsetsChange again.

Note this is driven by render count, not by anything actually changing. It fires even when onChange is a stable function, because the churn comes from SafeAreaListener's own inline arrow, not the caller's prop.

SafeAreaProvider is not affected - it already memoises its callback with React.useCallback(..., []) at src/SafeAreaContext.tsx:70. The path here is SafeAreaListener on web.

The fix

Store the latest onInsetsChange in a ref and drop it from the effect's dependencies, so setup runs once per mount. 10 lines of source change.

I considered fixing it in SafeAreaListener instead by wrapping its callback in useCallback([onChange]), and rejected it: that only helps callers who already memoise onChange, and an inline onChange - the common usage - would still churn. Fixing it in the web provider makes the effect correct for every caller. The effect body genuinely does not depend on the callback's identity, only on being able to call the current one, so the ref is the right tool here; there are no other values in the closure that would go stale (viewRef is a ref and createContextElement is module-level).

For contrast, the native NativeSafeAreaProvider passes onInsetsChange straight through to the native view, where a changing identity is just a prop update with no teardown - the web implementation is the only one holding resources across renders, so this stays scoped to the web file.

Test Plan

Reproduction

I added three tests to the existing src/__tests__/NativeSafeAreaProvider.web-test.tsx jsdom suite. The user-visible one renders a real SafeAreaListener with a stable onChange and re-renders it three times, then counts probe attachments, ResizeObserver constructions and measurement calls.

On main, after 3 renders:

before after
probe elements attached to document.body 3 1
ResizeObserver instances constructed 3 1
onChange calls 3 1

I have measured the setup/teardown counts above and nothing else - I am not claiming a frame-time or benchmark number.

Not regressing what the effect is for

Dropping a dependency risks a stale callback, so one of the tests guards exactly that: it mounts with callback A, re-renders with callback B (asserting B is not called just for being swapped in), then triggers a genuine resize and asserts B receives the new insets and frame while A is not called again. I verified this test fails if I take the dependency-array change without the ref, so the ref is load-bearing and not decoration. The existing tests covering window resize, ResizeObserver updates and the ResizeObserver-less fallback all still pass.

yarn test passes (prettier, eslint, tsc, jest: 28 tests, 11 snapshots, baseline on main is 25). The 3 eslint no-deep-imports warnings are pre-existing on main.

Possible overlap

Rebased onto main at 5.10.0. #775 landed in this file in the meantime and added a ref prop plus a setRef callback; that is a separate concern from the effect's dependency array, so the two sit side by side and the rendered tree is unchanged from main. The measurement effect still reads viewRef.current at mount, which is the same element setRef stores.

If an unstable_disableViewOnWeb prop for SafeAreaProvider (#637, #750) lands it also touches this file, but only the props destructure and the return statement, so the two do not overlap.

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.
@giaBaoJS
giaBaoJS force-pushed the fix/web-remeasure-on-every-render branch from 016ea7b to e39454a Compare September 17, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant