Skip to content

Commit 84ee4d4

Browse files
hoxyqmeta-codesync[bot]
authored andcommitted
Fix cascading update (#54438)
Summary: Pull Request resolved: #54438 # Changelog: [Internal] `LogBoxStateSubscription` subscribed to state updates in a mount effect. This callback could be called instantly on subscription initiation, which would cause a cascading update => setState in mount effect [1]. This diff changes the approach, the state is initialized before mount and the callback is not called synchronously on initialization. I had to keep `observe` function as it is right now, because it is used extensively in tests. Reviewed By: javache Differential Revision: D86449266 fbshipit-source-id: c7a2cffec760e697d8b5525e88997204b16fcd81
1 parent 2a1e12f commit 84ee4d4

1 file changed

Lines changed: 31 additions & 4 deletions

File tree

packages/react-native/Libraries/LogBox/Data/LogBoxData.js

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,21 @@ export function observe(observer: Observer): Subscription {
412412
};
413413
}
414414

415+
/**
416+
* Same as observe(), but doesn't call notify observer sync at the time of subscription.
417+
* Expected to be used only in LogBoxStateSubscription.
418+
*/
419+
function observeNext(observer: Observer): Subscription {
420+
const subscription = {observer};
421+
observers.add(subscription);
422+
423+
return {
424+
unsubscribe(): void {
425+
observers.delete(subscription);
426+
},
427+
};
428+
}
429+
415430
type LogBoxStateSubscriptionProps = $ReadOnly<{}>;
416431
type LogBoxStateSubscriptionState = $ReadOnly<{
417432
logs: LogBoxLogs,
@@ -447,12 +462,11 @@ export function withSubscription(
447462
}
448463

449464
_subscription: ?Subscription;
465+
_updateStateOnMountTimeoutId: ?TimeoutID;
450466

451467
state: LogBoxStateSubscriptionState = {
452-
logs: new Set(),
453-
isDisabled: false,
454468
hasError: false,
455-
selectedLogIndex: -1,
469+
...getNextState(),
456470
};
457471

458472
render(): React.Node {
@@ -472,12 +486,25 @@ export function withSubscription(
472486
}
473487

474488
componentDidMount(): void {
475-
this._subscription = observe(data => {
489+
this._subscription = observeNext(data => {
476490
this.setState(data);
477491
});
492+
493+
/**
494+
* This should cover the case when the state changes in between the first render and mount effect.
495+
* We defer the state update to next task to avoid cascading update.
496+
*/
497+
this._updateStateOnMountTimeoutId = setTimeout(() => {
498+
this._updateStateOnMountTimeoutId = null;
499+
this.setState(getNextState());
500+
}, 0);
478501
}
479502

480503
componentWillUnmount(): void {
504+
if (this._updateStateOnMountTimeoutId != null) {
505+
clearTimeout(this._updateStateOnMountTimeoutId);
506+
}
507+
481508
if (this._subscription != null) {
482509
this._subscription.unsubscribe();
483510
}

0 commit comments

Comments
 (0)