Skip to content

Commit a0680e8

Browse files
Joe Savonafacebook-github-bot
authored andcommitted
Fix useFragment w <Activity> a->b->a mutation edge case
Reviewed By: captbaritone Differential Revision: D81533833 fbshipit-source-id: d92ae8d2680dbf33a9ab26f7614f075733b5ea05
1 parent 7b0eeb0 commit a0680e8

File tree

1 file changed

+26
-9
lines changed

1 file changed

+26
-9
lines changed

packages/react-relay/relay-hooks/useFragmentInternal_EXPERIMENTAL.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -608,15 +608,20 @@ hook useFragmentInternal_EXPERIMENTAL(
608608
// or detaches (<Activity> going hidden), and then re-subscribes when the component
609609
// re-attaches (<Activity> going visible). These cases wouldn't fire the
610610
// "update" effect because the state and environment don't change.
611-
const storeSubscriptionRef = useRef<{
612-
dispose: () => void,
613-
selector: ?ReaderSelector,
614-
environment: IEnvironment,
615-
} | null>(null);
611+
const storeSubscriptionRef = useRef<
612+
| {
613+
kind: 'initialized',
614+
dispose: () => void,
615+
selector: ?ReaderSelector,
616+
environment: IEnvironment,
617+
}
618+
| {kind: 'missed-updates'}
619+
| {kind: 'uninitialized'},
620+
>({kind: 'uninitialized'});
616621
// $FlowFixMe[react-rule-hook] - the condition is static
617622
useEffect(() => {
618623
const storeSubscription = storeSubscriptionRef.current;
619-
if (storeSubscription != null) {
624+
if (storeSubscription.kind === 'initialized') {
620625
if (
621626
state.environment === storeSubscription.environment &&
622627
state.selector === storeSubscription.selector
@@ -626,6 +631,7 @@ hook useFragmentInternal_EXPERIMENTAL(
626631
} else {
627632
// The selector has changed, so we need to dispose of the previous subscription
628633
storeSubscription.dispose();
634+
storeSubscriptionRef.current = {kind: 'uninitialized'};
629635
}
630636
}
631637
if (state.kind === 'bailout') {
@@ -650,7 +656,11 @@ hook useFragmentInternal_EXPERIMENTAL(
650656
// to using the latest snapshot to subscribe.
651657
if (didMissUpdates) {
652658
setState(updatedState);
659+
storeSubscriptionRef.current = {kind: 'missed-updates'};
653660
// We missed updates, we're going to render again anyway so wait until then to subscribe
661+
// Setting the ref to kind: missed-updates ensures the second useEffect (simulating the
662+
// setup/teardown part of the crud effect) will not set up the subscription w the stale
663+
// state
654664
return;
655665
}
656666
stateForSubscription = updatedState;
@@ -661,24 +671,31 @@ hook useFragmentInternal_EXPERIMENTAL(
661671
setState,
662672
);
663673
storeSubscriptionRef.current = {
674+
kind: 'initialized',
664675
dispose,
665676
selector: state.selector,
666677
environment: state.environment,
667678
};
668679
}, [state]);
669680
// $FlowFixMe[react-rule-hook] - the condition is static
670681
useEffect(() => {
671-
if (storeSubscriptionRef.current == null && state.kind !== 'bailout') {
682+
if (
683+
storeSubscriptionRef.current.kind === 'uninitialized' &&
684+
state.kind !== 'bailout'
685+
) {
672686
const dispose = subscribeToSnapshot(state.environment, state, setState);
673687
storeSubscriptionRef.current = {
688+
kind: 'initialized',
674689
dispose,
675690
selector: state.selector,
676691
environment: state.environment,
677692
};
678693
}
679694
return () => {
680-
storeSubscriptionRef.current?.dispose();
681-
storeSubscriptionRef.current = null;
695+
if (storeSubscriptionRef.current.kind === 'initialized') {
696+
storeSubscriptionRef.current.dispose();
697+
}
698+
storeSubscriptionRef.current = {kind: 'uninitialized'};
682699
};
683700
// NOTE: this intentionally has no dependencies, see above comment about
684701
// simulating a CRUD effect

0 commit comments

Comments
 (0)