Skip to content

Commit

Permalink
Handle missed updates from the low-priority state changes in the new …
Browse files Browse the repository at this point in the history
…hooks implementation

Reviewed By: josephsavona

Differential Revision: D52541289

fbshipit-source-id: 6e2d35814e4b9138cf9c9027e6314fe2cc483109
  • Loading branch information
alunyov authored and facebook-github-bot committed Jan 4, 2024
1 parent aac57b3 commit 3873809
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 1 deletion.
52 changes: 52 additions & 0 deletions packages/react-relay/relay-hooks/__tests__/useFragmentNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,58 @@ describe.each([
});
});

it('should return the latest data when the hi-priority update happens at the same time as the low-priority store update', () => {
const startTransition = React.startTransition;
if (startTransition != null) {
internalAct(() => {
renderSingularFragment({
isConcurrent: true,
});
});
assertFragmentResults([
{
data: {
id: '1',
name: 'Alice',
profile_picture: null,
...createFragmentRef('1', singularQuery),
},
},
]);

internalAct(() => {
// Trigger store update with the lower priority
startTransition(() => {
environment.commitUpdate(store => {
store.get('1')?.setValue('Alice Updated Name', 'name');
});
});
// Trigger a hi-pri update with the higher priority, that should force component to re-render
forceSingularUpdate();
});

// Assert that the component re-renders twice, both times with the latest data
assertFragmentResults([
{
data: {
id: '1',
name: 'Alice Updated Name',
profile_picture: null,
...createFragmentRef('1', singularQuery),
},
},
{
data: {
id: '1',
name: 'Alice Updated Name',
profile_picture: null,
...createFragmentRef('1', singularQuery),
},
},
]);
}
});

it('should re-read and resubscribe to fragment when variables change', () => {
renderSingularFragment();
assertFragmentResults([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ function subscribeToSnapshot(
environment: IEnvironment,
state: FragmentState,
setState: StateUpdaterFunction<FragmentState>,
hasPendingStateChanges: {current: boolean},
): () => void {
if (state.kind === 'bailout') {
return () => {};
Expand All @@ -264,11 +265,14 @@ function subscribeToSnapshot(
name: 'useFragment.subscription.missedUpdates',
hasDataChanges: dataChanged,
});
hasPendingStateChanges.current = dataChanged;
return dataChanged ? nextState : prevState;
} else {
return prevState;
}
}

hasPendingStateChanges.current = true;
return {
kind: 'singular',
snapshot: latestSnapshot,
Expand Down Expand Up @@ -297,13 +301,16 @@ function subscribeToSnapshot(
name: 'useFragment.subscription.missedUpdates',
hasDataChanges: dataChanged,
});
hasPendingStateChanges.current =
hasPendingStateChanges.current || dataChanged;
return dataChanged ? nextState : prevState;
} else {
return prevState;
}
}
const updated = [...prevState.snapshots];
updated[index] = latestSnapshot;
hasPendingStateChanges.current = true;
return {
kind: 'plural',
snapshots: updated,
Expand Down Expand Up @@ -529,6 +536,8 @@ function useFragmentInternal_EXPERIMENTAL(
// they're missing even though we are out of options for possibly fetching them:
handlePotentialSnapshotErrorsForState(environment, state);

const hasPendingStateChanges = useRef<boolean>(false);

useEffect(() => {
// Check for updates since the state was rendered
let currentState = subscribedState;
Expand All @@ -547,9 +556,26 @@ function useFragmentInternal_EXPERIMENTAL(
}
currentState = updatedState;
}
return subscribeToSnapshot(environment, currentState, setState);
return subscribeToSnapshot(
environment,
currentState,
setState,
hasPendingStateChanges,
);
}, [environment, subscribedState]);

if (hasPendingStateChanges.current) {
const updates = handleMissedUpdates(environment, state);
if (updates != null) {
const [hasStateUpdates, updatedState] = updates;
if (hasStateUpdates) {
setState(updatedState);
state = updatedState;
}
}
hasPendingStateChanges.current = false;
}

let data: ?SelectorData | Array<?SelectorData>;
if (isPlural) {
// Plural fragments require allocating an array of the snapshot data values,
Expand Down

0 comments on commit 3873809

Please sign in to comment.