[DRAFT] Initial React "concurrent stores" compat prototype

[markerikson]

This PR:

• Is a first prototype to see if we can use the WIP React "concurrent stores" API as a replacement for useSyncExternalStore
  • Added a Yalc-built version of https://github.com/thejustinwalsh/react-concurrent-store that exposes a couple additional types to satisfy TS
  • Adds a copy of the reactStoreEnhancer from the polyfill tests
  • Updates <Provider> and ReactReduxContext to render the polyfill's <StoreProvider> component and the reactStore instance in context
  • Rewrites useSelector to call the new useStoreSelector hook instead of useSyncExternalStore, and adds wrapper logic that tries to backfill support for equality checks and unstable selector references (hopefully to be handled by the hook itself later on)
  • Adds a createTestStore util that configures the store with the React store enhancer
  • Updates useSelector.spec.tsx to use that createTestStore util for all of its stores
  • Updates useSelector.spec.tsx to mostly pass:
    • commented out now-broken assertions that accessed the React-Redux internal subscription counts, as the current implementation is now relying on the polyfill's StoreManager class to manage subscriptions
    • Disabled the "O(1) subscription removal" test, as StoreManager is using an array and not a linked list like our Subscription class
    • Updated all places where we get different selector call counts, which increased noticeably (1 -> 3, 2 -> 4, 4 -> 8, 3 -> 5, etc). Clearly the new implementation is calling selectors a lot more.

Background

@captbaritone and @thejustinwalsh have been working in https://github.com/thejustinwalsh/react-concurrent-store to prototype the upcoming React "concurrent stores" API. As a loose description, what we're hoping for is "useSyncExternalStore, minus the Sync" - aka a concurrent/transition-compatible way to integrate external state into React.

They've got a first POC polyfill up on NPM as react-concurrent-store, and the repo has a couple tiny demo integrations for Relay and Redux.

This is my attempt to do an experimental integration and rewrite React-Redux to use it and see how far we get, a la the alpha integration of useSyncExternalStore in #1808 .

Results

Impressively, especially for just a couple hours of hacking:

All but 3 useSelector tests pass!

The caveat is that I did change pretty much all places where we assert the number of selector calls (which all went up 2+ calls), and disabled a couple places where we asserted that we had N active internal subscriptions (as this mechanism bypasses our own Subscription implementation).

The test failures are:

• A test that asserts we always use the latest selector
• 2 tests that expect we handle errors thrown from selectors in a specific way

 FAIL  test/hooks/useSelector.spec.tsx > React > hooks > useSelector > uses the latest selector
AssertionError: expected [ +0, +0 ] to deeply equal [ +0, 1 ]

- Expected
+ Received

  Array [
    0,
-   1,
+   0,
  ]

 ❯ test/hooks/useSelector.spec.tsx:481:31
    479|           forceRender()
    480|         })
    481|         expect(renderedItems).toEqual([0, 1])
       |                               ^
    482|
    483|         rtl.act(() => {

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/3]⎯

 FAIL  test/hooks/useSelector.spec.tsx > React > hooks > useSelector > edge cases > ignores transient errors in selector (e.g. due to stale props)
AssertionError: expected [Function doDispatch] to not throw an error but 'Error' was thrown

- Expected:
undefined

+ Received:
"Error"

 ❯ test/hooks/useSelector.spec.tsx:528:34
    526|             })
    527|           }
    528|           expect(doDispatch).not.toThrowError()
       |                                  ^
    529|
    530|           spy.mockRestore()

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/3]⎯

 FAIL  test/hooks/useSelector.spec.tsx > React > hooks > useSelector > edge cases > re-throws errors from the selector that only occur during rendering
AssertionError: expected [Function] to throw an error
 ❯ test/hooks/useSelector.spec.tsx:602:14
    600|               normalStore.dispatch({ type: '' })
    601|             })
    602|           }).toThrowError()
       |              ^
    603|
    604|           spy.mockRestore()

Analysis

I believe our own v7 useSelector implementation would catch errors, swallow them, and force a re-render (under the assumption that if you had a stale props / "zombie child" scenario, then the problems would all go away by the time the component was done re-rendering). I don't know what the current uSES semantics are, but here useStoreSelector is clearly doing something different.

The selector stability test is obviously failing due to some combo of the actual useStoreSelector implementation not actually supporting unstable selector references and the attempt to make them work in userland here.

Overall, though, this is a pretty solid indication that we're on the right track already. Those are all pretty edge-case-y things, and the core functionality is working.

connect?

This does bring up a big question about if and how we'd be able to convert connect over. Personally I'd rather not :) connect has always been a big ugly beast of an implementation, and as it is I'm pretty sure the hacks I've got in place right now are an abuse of uSES's semantics. I don't even want to think about making that work with useStoreSelector. I'd be perfectly fine saying "NOPE, connect works as-is, with all its limitations, No Concurrent Behavior For You, go migrate to useSelector if you want that".

Store Enhancers?

You'll note this POC requires adding a Redux store enhancer to let us capture the action create the "React-compatible store":

export const addReactStore: StoreEnhancer<{
  reactStore: ReactStore
}> = (createStore) => {
  return (reducer, preloadedState) => {
    const store = createStore(reducer, preloadedState)

    // Create concurrent-safe store wrapper
    const reactStore = createStoreFromSource({
      getState: store.getState,
      reducer: reducer,
    })

    // Intercept dispatch to notify reactStore
    const originalDispatch = store.dispatch
    store.dispatch = (action: any) => {
      const result = originalDispatch(action)
      reactStore.handleUpdate(action)
      return result
    }

    // Attach reactStore to Redux store
    return Object.assign(store, { reactStore })
  }
}

It's hypothetically possible we might be able to find a different solution as we go, but as a first attempt this seems like the right integration point to me. This is something that would presumably be added after the middleware enhancer (and before the DevTools?), so that it only sees actions that are about to reach the reducer. (Actually, now that I say that... I don't know what happens when you try time-traveling in the DevTools, so that's something we should investigate.)

Subscriptions?

I realized this bypasses our own Subscription class entirely. That class has historically served 3 purposes:

• It enforced React batching pre-React-18
• It allowed us to do cascading updates in the commit phase, which was necessary to enforce connect's invariants that "mapState always sees the latest props from its parent"
• By using a linked list for subscribers instead of an array, we get O(1) performance for removing a subscriber ( Optimize createListenerCollection #1523 , Switch back to Subscription in useSelector to fix unsubscribe perf #1870 ). The polyfill's StoreManager class is using an array, so it presumably has that same perf problem. (I looked at uSES, and technically I don't think it actually tracks any subscribers itself?

Some of the tests failed because they asserted there were N active subscriptions, and now we don't have any subscriptions ourselves at all.

I'm not sure what happens if you were to mix-and-match at this point. I assume that a connect in the tree would still get triggered from Subscription.

To some extent StoreManager is an alternative to Subscription. Probably ought to use a linked list there if it's going to track subscribers, though.

Next Steps

useStoreSelector Changes

Per thejustinwalsh/react-concurrent-store#13 we really need this to allow unstable selector references, because this is a standard ecosystem pattern:

const todo = useSelector(state => selectTodoById(state, id))

We also need to handle customizable equality checks. Currently we rely on useSyncExternalStoreWithSelector supporting that:

• useSyncExternalStoreWithSelector.js: isEqual

useStoreSelector will probably need to do something similar.

There's also the error-handling semantics question.

Testing Scenarios

Beyond that, though, we'd want to start adding some tests with transition behaviors and see what happens.

We'd also want to do some testing with Redux middleware and the Redux DevTools and see what happens and what expectations might break - certainly in normal sync mode, then with transitions added in.

[captbaritone]

Connect

I think leaving them behind is probably fine for now. We can revisit if we get significant pushback.

Store Enhancers

Agree, next step is to pressure test some enhancers combinations and validate that they behave as expected.

Subscriptions

My implementation was just written to get it working quickly. Would love to update the subscription code to have more sensible perf characteristics. Linked list sounds like a good solution! Would you like to submit a PR?

useStoreSelector Changes

Unstable Selectors

I'm working on thejustinwalsh/react-concurrent-store#13

isEqual

Rather than supporting isEqual I'd like to explore passing the selector the current and previous values. This would allow libraries like Relay to implement recycleNodesInto which allows selectors which are pure functions to end up returning objects which implement structural sharing, which is important for keeping memoization in React happy. (Example, your selector returns a nested object and a state change causes you to return a new top level field, but the nested items have all the same values. This allows you to keep stable object identity for those nested objects.

I think this can be used to implement a custom isEqual.

Error handling

Are the error handling semantics of Redux selectors documented anywhere? Because selectors are eager, there's a zombie child problem where you may evaluate a selector which is not actually expected to be read in the new state, so we may want to swallow those errors, similar to how React swallows errors in updater functions.

[markerikson]

I can imagine that passing selector(currState, prevState) might run into assumptions about selector arguments, but would have to come up with concrete examples.

The main bit of documentation we've had around "zombie children" and selector errors is here:

• https://react-redux.js.org/api/hooks#stale-props-and-zombie-children

useSelector() tries to deal with this by catching all errors that are thrown when the selector is executed due to a store update (but not when it is executed during rendering). When an error occurs, the component will be forced to render, at which point the selector is executed again. This works as long as the selector is a pure function and you do not depend on the selector throwing errors.

I probably haven't updated that paragraph since before we switched to useSyncExternalStore, so the listed semantics may be outdated vs whatever uSES does.

Also a bit of historical context here:

• https://blog.isquaredsoftware.com/2018/11/react-redux-history-implementation/#v7-1-hooks

[captbaritone]

Re selector arguments, I think from React's perspective, there are no selector arguments. They must be pre-bound into the selector, or use a "higher order selector" where the selector returns a partially applied function which accepts the arguments: (state) => (...args) => T