Skip to content

Commit f2c3811

Browse files
authored
fix: useSyncExternalStoreExtra (#22500)
* move setting memoizedSnapshot * Revert "move setting memoizedSnapshot" This reverts commit f013206. * add failed tests * Revert "Revert "move setting memoizedSnapshot"" This reverts commit cb43c4f.
1 parent 0ecbbe1 commit f2c3811

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

Diff for: packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js

+89
Original file line numberDiff line numberDiff line change
@@ -818,4 +818,93 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
818818
'Sibling: 1',
819819
]);
820820
});
821+
822+
describe('selector and isEqual error handling in extra', () => {
823+
let ErrorBoundary;
824+
beforeAll(() => {
825+
spyOnDev(console, 'warn');
826+
ErrorBoundary = class extends React.Component {
827+
state = {error: null};
828+
static getDerivedStateFromError(error) {
829+
return {error};
830+
}
831+
render() {
832+
if (this.state.error) {
833+
return <Text text={this.state.error.message} />;
834+
}
835+
return this.props.children;
836+
}
837+
};
838+
});
839+
840+
it('selector can throw on update', async () => {
841+
const store = createExternalStore({a: 'a'});
842+
const selector = state => state.a.toUpperCase();
843+
844+
function App() {
845+
const a = useSyncExternalStoreExtra(
846+
store.subscribe,
847+
store.getState,
848+
null,
849+
selector,
850+
);
851+
return <Text text={a} />;
852+
}
853+
854+
const container = document.createElement('div');
855+
const root = createRoot(container);
856+
await act(() =>
857+
root.render(
858+
<ErrorBoundary>
859+
<App />
860+
</ErrorBoundary>,
861+
),
862+
);
863+
864+
expect(container.textContent).toEqual('A');
865+
866+
await act(() => {
867+
store.set({});
868+
});
869+
expect(container.textContent).toEqual(
870+
"Cannot read property 'toUpperCase' of undefined",
871+
);
872+
});
873+
874+
it('isEqual can throw on update', async () => {
875+
const store = createExternalStore({a: 'A'});
876+
const selector = state => state.a;
877+
const isEqual = (left, right) => left.a.trim() === right.a.trim();
878+
879+
function App() {
880+
const a = useSyncExternalStoreExtra(
881+
store.subscribe,
882+
store.getState,
883+
null,
884+
selector,
885+
isEqual,
886+
);
887+
return <Text text={a} />;
888+
}
889+
890+
const container = document.createElement('div');
891+
const root = createRoot(container);
892+
await act(() =>
893+
root.render(
894+
<ErrorBoundary>
895+
<App />
896+
</ErrorBoundary>,
897+
),
898+
);
899+
900+
expect(container.textContent).toEqual('A');
901+
902+
await act(() => {
903+
store.set({});
904+
});
905+
expect(container.textContent).toEqual(
906+
"Cannot read property 'trim' of undefined",
907+
);
908+
});
909+
});
821910
});

Diff for: packages/use-sync-external-store/src/useSyncExternalStoreExtra.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
7676
}
7777

7878
// The snapshot has changed, so we need to compute a new selection.
79-
memoizedSnapshot = nextSnapshot;
8079
const nextSelection = selector(nextSnapshot);
8180

8281
// If a custom isEqual function is provided, use that to check if the data
@@ -87,6 +86,7 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
8786
return prevSelection;
8887
}
8988

89+
memoizedSnapshot = nextSnapshot;
9090
memoizedSelection = nextSelection;
9191
return nextSelection;
9292
};

0 commit comments

Comments
 (0)