Repository: facebook/react
Pull request: fix: useSyncExternalStoreExtra

<!--
  Thanks for submitting a pull request!
  We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory.

  Before submitting a pull request, please make sure the following is done:

  1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`.
  2. Run `yarn` in the repository root.
  3. If you've fixed a bug or added code that should be tested, add tests!
  4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development.
  5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`.
  6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect".
  7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`).
  8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

  Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html
-->

## Summary

Discussion from: https://github.com/reactwg/react-18/discussions/86#discussioncomment-1422178

This change is to fix the case when selector and isEqual throw.

- [x] initial fix to see if CI passes
- [x] add failing test
- [x] finalize PR

<!--
 Explain the **motivation** for making this change. What existing problem does the pull request solve?
-->

## How did you test this change?

<!--
  Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface.
  How exactly did you verify that your PR solves the issue you wanted to solve?
  If you leave this empty, your PR will very likely be closed.
-->

Test added.

Changed files:
filename: packages/use-sync-external-store/src/__tests__/useSyncExternalStoreShared-test.js
patch: @@ -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 <Text text={this.state.error.message} />;
+          }
+          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 <Text text={a} />;
+      }
+
+      const container = document.createElement('div');
+      const root = createRoot(container);
+      await act(() =>
+        root.render(
+          <ErrorBoundary>
+            <App />
+          </ErrorBoundary>,
+        ),
+      );
+
+      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 <Text text={a} />;
+      }
+
+      const container = document.createElement('div');
+      const root = createRoot(container);
+      await act(() =>
+        root.render(
+          <ErrorBoundary>
+            <App />
+          </ErrorBoundary>,
+        ),
+      );
+
+      expect(container.textContent).toEqual('A');
+
+      await act(() => {
+        store.set({});
+      });
+      expect(container.textContent).toEqual(
+        "Cannot read property 'trim' of undefined",
+      );
+    });
+  });
 });

filename: packages/use-sync-external-store/src/useSyncExternalStoreExtra.js
patch: @@ -76,7 +76,6 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
       }
 
       // 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<Snapshot, Selection>(
         return prevSelection;
       }
 
+      memoizedSnapshot = nextSnapshot;
       memoizedSelection = nextSelection;
       return nextSelection;
     };