Skip to content

Commit a59f60a

Browse files
Warn in development when a view reports its safe area insets in a loop
The system UI does not move many times a second, so a sustained stream of inset events means the layout is feeding the insets back into the position of the observed view: it is offset by the insets it reports, which moves it out from under the system UI, which changes its insets. Every one of those events renders synchronously, so the loop is paid for in frames. `View` wraps the handler in development builds and warns once per view above ten events in a second. The check lives in the handler `View` passes down rather than in either platform's observer, so it covers iOS and Android with one implementation and surfaces in LogBox with a JavaScript stack. The production branch is the identity function, so the module stays out of the bundle, and the native prop is unaffected either way — function props are normalized to `true` before props are diffed, so wrapping does not produce an update. Counts are kept per view in a `WeakMap` keyed by the event target, so views that do not loop are never charged for it. RNTester grows the mistake it warns about, and a Fantom test with a mocked clock covers the rate, the once-per-view behaviour, per-view counting, and that the handler still receives its event.
1 parent e71752a commit a59f60a

4 files changed

Lines changed: 308 additions & 0 deletions

File tree

packages/react-native/Libraries/Components/View/View.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,23 @@
99
*/
1010

1111
import type {HostInstance} from '../../../src/private/types/HostInstance';
12+
import type {SafeAreaInsetsChangeEvent} from '../../Types/CoreEventTypes';
1213
import type {ViewProps} from './ViewPropTypes';
1314

1415
import TextAncestorContext from '../../Text/TextAncestorContext';
1516
import ViewNativeComponent from './ViewNativeComponent';
1617
import * as React from 'react';
1718
import {use} from 'react';
1819

20+
// Only development builds check for a view reporting its insets in a loop; the
21+
// production branch keeps the handler as it is, and the module out of the bundle.
22+
const warnOnRepeatedSafeAreaInsetsChanges: (
23+
onSafeAreaInsetsChange: (event: SafeAreaInsetsChangeEvent) => unknown,
24+
) => (event: SafeAreaInsetsChangeEvent) => unknown = __DEV__
25+
? require('../../../src/private/components/view/warnOnRepeatedSafeAreaInsetsChanges')
26+
.default
27+
: onSafeAreaInsetsChange => onSafeAreaInsetsChange;
28+
1929
export type ViewInstance = HostInstance;
2030

2131
/**
@@ -115,6 +125,18 @@ component View(ref?: React.RefSetter<ViewInstance>, ...props: ViewProps) {
115125
};
116126
}
117127

128+
if (__DEV__) {
129+
// Views are the only place the prop is used in practice, so the check for a
130+
// view reporting its insets in a loop lives here rather than on every host
131+
// component that inherits the prop.
132+
const onSafeAreaInsetsChange =
133+
resolvedProps.experimental_onSafeAreaInsetsChange;
134+
if (onSafeAreaInsetsChange != null) {
135+
resolvedProps.experimental_onSafeAreaInsetsChange =
136+
warnOnRepeatedSafeAreaInsetsChanges(onSafeAreaInsetsChange);
137+
}
138+
}
139+
118140
const actualView =
119141
ref == null ? (
120142
<ViewNativeComponent {...resolvedProps} />
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
import '@react-native/fantom/src/setUpDefaultReactNativeEnvironment';
12+
13+
import type {HighResTimeStampMock} from '@react-native/fantom/src/HighResTimeStampMock';
14+
import type {HostInstance} from 'react-native/src/private/types/HostInstance';
15+
16+
import * as Fantom from '@react-native/fantom';
17+
import * as React from 'react';
18+
import {createRef} from 'react';
19+
import {View} from 'react-native';
20+
21+
const INSETS = {top: 44, right: 0, bottom: 34, left: 0};
22+
const FRAME = {x: 0, y: 0, width: 390, height: 844};
23+
24+
function renderObservingView(): {current: HostInstance | null} {
25+
const nodeRef = createRef<HostInstance>();
26+
const root = Fantom.createRoot();
27+
Fantom.runTask(() => {
28+
root.render(
29+
<View ref={nodeRef} experimental_onSafeAreaInsetsChange={() => {}} />,
30+
);
31+
});
32+
return nodeRef;
33+
}
34+
35+
function dispatchInsetsChange(nodeRef: {current: HostInstance | null}) {
36+
Fantom.dispatchNativeEvent(nodeRef, 'safeAreaInsetsChange', {
37+
insets: INSETS,
38+
frame: FRAME,
39+
});
40+
}
41+
42+
describe('experimental_onSafeAreaInsetsChange warning', () => {
43+
const originalConsoleWarn = console.warn;
44+
let mockConsoleWarn: JestMockFn<ReadonlyArray<unknown>, void>;
45+
let mockClock: ?HighResTimeStampMock;
46+
47+
beforeEach(() => {
48+
mockConsoleWarn = jest.fn();
49+
// $FlowFixMe[cannot-write]
50+
console.warn = mockConsoleWarn;
51+
mockClock = Fantom.installHighResTimeStampMock();
52+
});
53+
54+
afterEach(() => {
55+
// $FlowFixMe[cannot-write]
56+
console.warn = originalConsoleWarn;
57+
mockClock?.uninstall();
58+
mockClock = null;
59+
});
60+
61+
it('stays silent while the insets change at a plausible rate', () => {
62+
const nodeRef = renderObservingView();
63+
64+
// A rotation, a keyboard, a split view: a handful of changes, spread out.
65+
for (let i = 0; i < 20; i++) {
66+
dispatchInsetsChange(nodeRef);
67+
mockClock?.advanceTimeBy(200);
68+
}
69+
70+
expect(mockConsoleWarn).not.toHaveBeenCalled();
71+
});
72+
73+
it('warns once when a single view loops within the window', () => {
74+
const nodeRef = renderObservingView();
75+
76+
for (let i = 0; i < 11; i++) {
77+
dispatchInsetsChange(nodeRef);
78+
mockClock?.advanceTimeBy(16);
79+
}
80+
81+
expect(mockConsoleWarn).toHaveBeenCalledTimes(1);
82+
expect(mockConsoleWarn.mock.lastCall[0]).toContain(
83+
'`experimental_onSafeAreaInsetsChange` fired more than 10 times in 1000ms',
84+
);
85+
86+
// The loop keeps running; the warning does not.
87+
for (let i = 0; i < 50; i++) {
88+
dispatchInsetsChange(nodeRef);
89+
mockClock?.advanceTimeBy(16);
90+
}
91+
92+
expect(mockConsoleWarn).toHaveBeenCalledTimes(1);
93+
});
94+
95+
it('counts each view separately', () => {
96+
const nodeRefA = renderObservingView();
97+
const nodeRefB = renderObservingView();
98+
99+
for (let i = 0; i < 10; i++) {
100+
dispatchInsetsChange(nodeRefA);
101+
dispatchInsetsChange(nodeRefB);
102+
mockClock?.advanceTimeBy(16);
103+
}
104+
105+
expect(mockConsoleWarn).not.toHaveBeenCalled();
106+
107+
dispatchInsetsChange(nodeRefA);
108+
109+
expect(mockConsoleWarn).toHaveBeenCalledTimes(1);
110+
});
111+
112+
it('still delivers the event to the handler', () => {
113+
const nodeRef = createRef<HostInstance>();
114+
const onSafeAreaInsetsChange = jest.fn();
115+
const root = Fantom.createRoot();
116+
Fantom.runTask(() => {
117+
root.render(
118+
<View
119+
ref={nodeRef}
120+
experimental_onSafeAreaInsetsChange={event => {
121+
onSafeAreaInsetsChange(event.nativeEvent);
122+
}}
123+
/>,
124+
);
125+
});
126+
127+
dispatchInsetsChange(nodeRef);
128+
129+
expect(onSafeAreaInsetsChange).toHaveBeenCalledTimes(1);
130+
expect(onSafeAreaInsetsChange.mock.lastCall[0].insets).toEqual(INSETS);
131+
});
132+
});
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
import type {SafeAreaInsetsChangeEvent} from '../../../../Libraries/Types/CoreEventTypes';
12+
13+
const DISPATCH_WINDOW_MS = 1000;
14+
const MAX_DISPATCHES_PER_WINDOW = 10;
15+
16+
type DispatchRate = {
17+
count: number,
18+
windowStart: number,
19+
warned: boolean,
20+
};
21+
22+
const dispatchRates: WeakMap<interface {}, DispatchRate> = new WeakMap();
23+
24+
/**
25+
* Wraps an `experimental_onSafeAreaInsetsChange` handler with a development
26+
* check for a view that reports insets over and over.
27+
*
28+
* The system UI does not move many times a second, so a sustained stream of
29+
* events means the layout is feeding the insets back into the position of the
30+
* observed view: it pads itself by the insets it reports, which moves it, which
31+
* changes its insets. Every one of those events renders synchronously, blocking
32+
* the UI thread, so the loop is paid for in frames.
33+
*/
34+
export default function warnOnRepeatedSafeAreaInsetsChanges(
35+
onSafeAreaInsetsChange: (event: SafeAreaInsetsChangeEvent) => unknown,
36+
): (event: SafeAreaInsetsChangeEvent) => unknown {
37+
return event => {
38+
// The target identifies the view without keeping it alive; events dispatched
39+
// without one are simply not counted.
40+
const target = event.target;
41+
if (target != null && typeof target === 'object') {
42+
warnIfDispatchingTooOften(target);
43+
}
44+
return onSafeAreaInsetsChange(event);
45+
};
46+
}
47+
48+
function warnIfDispatchingTooOften(target: interface {}): void {
49+
const now = performance.now();
50+
let dispatchRate: ?DispatchRate = dispatchRates.get(target);
51+
if (dispatchRate == null) {
52+
const newDispatchRate: DispatchRate = {
53+
count: 0,
54+
windowStart: now,
55+
warned: false,
56+
};
57+
dispatchRates.set(target, newDispatchRate);
58+
dispatchRate = newDispatchRate;
59+
}
60+
if (dispatchRate.warned) {
61+
return;
62+
}
63+
if (now - dispatchRate.windowStart > DISPATCH_WINDOW_MS) {
64+
dispatchRate.windowStart = now;
65+
dispatchRate.count = 0;
66+
}
67+
dispatchRate.count++;
68+
if (dispatchRate.count > MAX_DISPATCHES_PER_WINDOW) {
69+
dispatchRate.warned = true;
70+
console.warn(
71+
`\`experimental_onSafeAreaInsetsChange\` fired more than ${MAX_DISPATCHES_PER_WINDOW} ` +
72+
`times in ${DISPATCH_WINDOW_MS}ms on a single view. The safe area insets of a view ` +
73+
'only change when the system UI moves or the view does, so this is usually a loop: ' +
74+
'the view is laid out from the insets it reports, which moves it, which changes its ' +
75+
'insets. Each event renders synchronously, so the loop costs frames.',
76+
);
77+
}
78+
}

packages/rn-tester/js/examples/SafeAreaInsets/SafeAreaInsetsExample.js

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,71 @@ function FullScreenExample(): React.Node {
147147
);
148148
}
149149

150+
function FeedbackLoopModalContent({
151+
onClose,
152+
}: {
153+
onClose: () => void,
154+
}): React.Node {
155+
const [insets, , onSafeAreaInsetsChange] = useSafeAreaInsets();
156+
const [eventCount, setEventCount] = useState(0);
157+
const [looping, setLooping] = useState(false);
158+
159+
const onLoopingInsetsChange = useCallback(
160+
(event: SafeAreaInsetsChangeEvent) => {
161+
setEventCount(count => count + 1);
162+
onSafeAreaInsetsChange(event);
163+
},
164+
[onSafeAreaInsetsChange],
165+
);
166+
167+
return (
168+
<View style={styles.modal}>
169+
{/*
170+
The mistake this warns about: the view is *positioned* by the insets it
171+
reports, instead of padded by them. Offsetting it moves it out from
172+
under the system UI, which zeroes its insets, which moves it back.
173+
*/}
174+
<View
175+
experimental_onSafeAreaInsetsChange={
176+
looping ? onLoopingInsetsChange : undefined
177+
}
178+
style={[styles.loopBox, {marginTop: looping ? (insets?.top ?? 0) : 0}]}>
179+
<RNTesterText>{`inset events: ${eventCount}`}</RNTesterText>
180+
</View>
181+
<View style={styles.modalContent}>
182+
<RNTesterText>
183+
Starting the loop should log a development warning after ten events in
184+
a second, once, while the counter keeps climbing.
185+
</RNTesterText>
186+
{!looping ? (
187+
<Button onPress={() => setLooping(true)} title="Start the loop" />
188+
) : null}
189+
<Button onPress={onClose} title="Close" />
190+
</View>
191+
</View>
192+
);
193+
}
194+
195+
function FeedbackLoopExample(): React.Node {
196+
const [modalVisible, setModalVisible] = useState(false);
197+
198+
return (
199+
<View>
200+
<Modal
201+
visible={modalVisible}
202+
onRequestClose={() => setModalVisible(false)}
203+
animationType="slide"
204+
supportedOrientations={['portrait', 'landscape']}>
205+
<FeedbackLoopModalContent onClose={() => setModalVisible(false)} />
206+
</Modal>
207+
<Button
208+
onPress={() => setModalVisible(true)}
209+
title="Present a view that loops"
210+
/>
211+
</View>
212+
);
213+
}
214+
150215
let benchEventCount = 0;
151216

152217
function BenchRow({
@@ -274,6 +339,11 @@ const styles = StyleSheet.create({
274339
flexDirection: 'row',
275340
flexWrap: 'wrap',
276341
},
342+
loopBox: {
343+
backgroundColor: '#ffd7d7',
344+
paddingVertical: 8,
345+
paddingHorizontal: 12,
346+
},
277347
benchScrollContainer: {
278348
height: 300,
279349
borderWidth: 1,
@@ -307,6 +377,12 @@ exports.examples = [
307377
'A full screen view that pads itself by its own safe area insets.',
308378
render: (): React.Node => <FullScreenExample />,
309379
},
380+
{
381+
title: 'A view that reports its insets in a loop',
382+
description:
383+
'A view positioned by the insets it reports, which moves it out of the system UI and back. Development builds warn once when a view does this.',
384+
render: (): React.Node => <FeedbackLoopExample />,
385+
},
310386
{
311387
title: 'Scroll benchmark',
312388
description:

0 commit comments

Comments
 (0)