@samitouri / QOS-React-2 / commits / fd5e01c2e0

[useSES/extra] Reuse old selection if possible (#22307)

When you pass an unmemoized selector to useSyncExternalStoreExtra, we have to reevaluate it on every render because we don't know whether it depends on new props. However, after reevalutating it, we should still compare the result to the previous selection with `isEqual` and reuse the old one if it hasn't changed. Originally I did not implement this, because if the selector changes due to new props or state, the component is going to have to re-render anyway. However, it's still useful to return a memoized selection when possible, because it may be the input to a downstream memoization. In the test I wrote, the example I chose is selecting a list of items from the store, and passing the list as a prop to a memoized component. If the list prop is memoized, we can bail out.

Andrew Clark committed Sep 13, 2021 at 17:26 UTC fd5e01c2e0dbbeaff954d13fc6bc11bfc65e7dcf
2 files changed +114 -1
packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
+81
@@ -571,6 +571,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
571 });
572
573 describe('extra features implemented in user-space', () => {
574 + // The selector implementation uses the lazy ref initialization pattern
575 + // @gate !(enableUseRefAccessWarning && __DEV__)
576 test('memoized selectors are only called once per update', async () => {
577 const store = createExternalStore({a: 0, b: 0});
578
@@ -610,6 +612,8 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
612 expect(root).toMatchRenderedOutput('A1');
613 });
614
615 + // The selector implementation uses the lazy ref initialization pattern
616 + // @gate !(enableUseRefAccessWarning && __DEV__)
617 test('Using isEqual to bailout', async () => {
618 const store = createExternalStore({a: 0, b: 0});
619
@@ -666,4 +670,81 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
670 expect(root).toMatchRenderedOutput('A1B1');
671 });
672 });
673 +
674 + // The selector implementation uses the lazy ref initialization pattern
675 + // @gate !(enableUseRefAccessWarning && __DEV__)
676 + test('compares selection to rendered selection even if selector changes', async () => {
677 + const store = createExternalStore({items: ['A', 'B']});
678 +
679 + const shallowEqualArray = (a, b) => {
680 + if (a.length !== b.length) {
681 + return false;
682 + }
683 + for (let i = 0; i < a.length; i++) {
684 + if (a[i] !== b[i]) {
685 + return false;
686 + }
687 + }
688 + return true;
689 + };
690 +
691 + const List = React.memo(({items}) => {
692 + return (
693 + <ul>
694 + {items.map(text => (
695 + <li key={text}>
696 + <Text key={text} text={text} />
697 + </li>
698 + ))}
699 + </ul>
700 + );
701 + });
702 +
703 + function App({step}) {
704 + const inlineSelector = state => {
705 + Scheduler.unstable_yieldValue('Inline selector');
706 + return [...state.items, 'C'];
707 + };
708 + const items = useSyncExternalStoreExtra(
709 + store.subscribe,
710 + store.getState,
711 + inlineSelector,
712 + shallowEqualArray,
713 + );
714 + return (
715 + <>
716 + <List items={items} />
717 + <Text text={'Sibling: ' + step} />
718 + </>
719 + );
720 + }
721 +
722 + const root = createRoot();
723 + await act(() => {
724 + root.render(<App step={0} />);
725 + });
726 + expect(Scheduler).toHaveYielded([
727 + 'Inline selector',
728 + 'A',
729 + 'B',
730 + 'C',
731 + 'Sibling: 0',
732 + ]);
733 +
734 + await act(() => {
735 + root.render(<App step={1} />);
736 + });
737 + expect(Scheduler).toHaveYielded([
738 + // We had to call the selector again because it's not memoized
739 + 'Inline selector',
740 +
741 + // But because the result was the same (according to isEqual) we can
742 + // bail out of rendering the memoized list. These are skipped:
743 + // 'A',
744 + // 'B',
745 + // 'C',
746 +
747 + 'Sibling: 1',
748 + ]);
749 + });
750 });
packages/use-sync-external-store/src/useSyncExternalStoreExtra.js
+33 -1
@@ -13,7 +13,7 @@ import {useSyncExternalStore} from 'use-sync-external-store';
13
14 // Intentionally not using named imports because Rollup uses dynamic
15 // dispatch for CommonJS interop named imports.
16 -const {useMemo, useDebugValue} = React;
16 +const {useRef, useEffect, useMemo, useDebugValue} = React;
17
18 // Same as useSyncExternalStore, but supports selector and isEqual arguments.
19 export function useSyncExternalStoreExtra<Snapshot, Selection>(
@@ -22,6 +22,19 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
22 selector: (snapshot: Snapshot) => Selection,
23 isEqual?: (a: Selection, b: Selection) => boolean,
24 ): Selection {
25 + // Use this to track the rendered snapshot.
26 + const instRef = useRef(null);
27 + let inst;
28 + if (instRef.current === null) {
29 + inst = {
30 + hasValue: false,
31 + value: (null: Selection | null),
32 + };
33 + instRef.current = inst;
34 + } else {
35 + inst = instRef.current;
36 + }
37 +
38 const getSnapshotWithMemoizedSelector = useMemo(() => {
39 // Track the memoized state using closure variables that are local to this
40 // memoized instance of a getSnapshot function. Intentionally not using a
@@ -38,6 +51,18 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
51 hasMemo = true;
52 memoizedSnapshot = nextSnapshot;
53 const nextSelection = selector(nextSnapshot);
54 + if (isEqual !== undefined) {
55 + // Even if the selector has changed, the currently rendered selection
56 + // may be equal to the new selection. We should attempt to reuse the
57 + // current value if possible, to preserve downstream memoizations.
58 + if (inst.hasValue) {
59 + const currentSelection = inst.value;
60 + if (isEqual(currentSelection, nextSelection)) {
61 + memoizedSelection = currentSelection;
62 + return currentSelection;
63 + }
64 + }
65 + }
66 memoizedSelection = nextSelection;
67 return nextSelection;
68 }
@@ -67,10 +92,17 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
92 return nextSelection;
93 };
94 }, [getSnapshot, selector, isEqual]);
95 +
96 const value = useSyncExternalStore(
97 subscribe,
98 getSnapshotWithMemoizedSelector,
99 );
100 +
101 + useEffect(() => {
102 + inst.hasValue = true;
103 + inst.value = value;
104 + }, [value]);
105 +
106 useDebugValue(value);
107 return value;
108 }