From f01320689cebfcbc4f3a53208f879ed4a8d6613d Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 4 Oct 2021 11:17:38 +0900 Subject: [PATCH 1/4] move setting memoizedSnapshot --- .../use-sync-external-store/src/useSyncExternalStoreExtra.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js index f4a1885aec5b9..aa4957b534753 100644 --- a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js +++ b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js @@ -76,7 +76,6 @@ export function useSyncExternalStoreExtra( } // The snapshot has changed, so we need to compute a new selection. - memoizedSnapshot = nextSnapshot; const nextSelection = selector(nextSnapshot); // If a custom isEqual function is provided, use that to check if the data @@ -87,6 +86,7 @@ export function useSyncExternalStoreExtra( return prevSelection; } + memoizedSnapshot = nextSnapshot; memoizedSelection = nextSelection; return nextSelection; }; From cb43c4fdc6b0dcab3480f27d6fbbb3137dbc47bb Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 4 Oct 2021 13:49:04 +0900 Subject: [PATCH 2/4] Revert "move setting memoizedSnapshot" This reverts commit f01320689cebfcbc4f3a53208f879ed4a8d6613d. --- .../use-sync-external-store/src/useSyncExternalStoreExtra.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js index aa4957b534753..f4a1885aec5b9 100644 --- a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js +++ b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js @@ -76,6 +76,7 @@ export function useSyncExternalStoreExtra( } // The snapshot has changed, so we need to compute a new selection. + memoizedSnapshot = nextSnapshot; const nextSelection = selector(nextSnapshot); // If a custom isEqual function is provided, use that to check if the data @@ -86,7 +87,6 @@ export function useSyncExternalStoreExtra( return prevSelection; } - memoizedSnapshot = nextSnapshot; memoizedSelection = nextSelection; return nextSelection; }; From 3fa565a149e7efd5dfe25c0441f6ae23702ce735 Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 4 Oct 2021 16:11:15 +0900 Subject: [PATCH 3/4] add failed tests --- .../useSyncExternalStoreShared-test.js | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js index 6536f65b7f2a2..ab8143f24034c 100644 --- a/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js +++ b/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js @@ -818,4 +818,93 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => { 'Sibling: 1', ]); }); + + describe('selector and isEqual error handling in extra', () => { + let ErrorBoundary; + beforeAll(() => { + spyOnDev(console, 'warn'); + ErrorBoundary = class extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + if (this.state.error) { + return ; + } + return this.props.children; + } + }; + }); + + it('selector can throw on update', async () => { + const store = createExternalStore({a: 'a'}); + const selector = state => state.a.toUpperCase(); + + function App() { + const a = useSyncExternalStoreExtra( + store.subscribe, + store.getState, + null, + selector, + ); + return ; + } + + const container = document.createElement('div'); + const root = createRoot(container); + await act(() => + root.render( + + + , + ), + ); + + expect(container.textContent).toEqual('A'); + + await act(() => { + store.set({}); + }); + expect(container.textContent).toEqual( + "Cannot read property 'toUpperCase' of undefined", + ); + }); + + it('isEqual can throw on update', async () => { + const store = createExternalStore({a: 'A'}); + const selector = state => state.a; + const isEqual = (left, right) => left.a.trim() === right.a.trim(); + + function App() { + const a = useSyncExternalStoreExtra( + store.subscribe, + store.getState, + null, + selector, + isEqual, + ); + return ; + } + + const container = document.createElement('div'); + const root = createRoot(container); + await act(() => + root.render( + + + , + ), + ); + + expect(container.textContent).toEqual('A'); + + await act(() => { + store.set({}); + }); + expect(container.textContent).toEqual( + "Cannot read property 'trim' of undefined", + ); + }); + }); }); From f3eecd8bcac1b3a117c198ff441e97934b405b40 Mon Sep 17 00:00:00 2001 From: daishi Date: Mon, 4 Oct 2021 16:20:13 +0900 Subject: [PATCH 4/4] Revert "Revert "move setting memoizedSnapshot"" This reverts commit cb43c4fdc6b0dcab3480f27d6fbbb3137dbc47bb. --- .../use-sync-external-store/src/useSyncExternalStoreExtra.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js index f4a1885aec5b9..aa4957b534753 100644 --- a/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js +++ b/packages/use-sync-external-store/src/useSyncExternalStoreExtra.js @@ -76,7 +76,6 @@ export function useSyncExternalStoreExtra( } // The snapshot has changed, so we need to compute a new selection. - memoizedSnapshot = nextSnapshot; const nextSelection = selector(nextSnapshot); // If a custom isEqual function is provided, use that to check if the data @@ -87,6 +86,7 @@ export function useSyncExternalStoreExtra( return prevSelection; } + memoizedSnapshot = nextSnapshot; memoizedSelection = nextSelection; return nextSelection; };