Feedback on `useMutableSource`

_intro.md

Dear reader,

In the last few years, I've been using my own global store implementation with great success (yes, I'm aware there exist a bazillion "redux like" stores). My one's called tiny-atom.

In particular, the following 4 requirements for a global store emerged from my needs:

1. Batch multiple, rapid store changes

For example, tiny-atom uses requestAnimationFrame to delay/batch the re-renders.

2. Do not re-render children twice on each store change

If naively implemented – a child component re-renders once from parent re-render, once from it's own subscription.

3. Re-render parent first to avoid "zombie children"

For example, a child component can break reading something non existant from store, when parent was supposed to un-render that child.

4. Only re-render if selected slice of state changed

Important for performance, when many components bind to small slices of the shared state.

My current implementation is not Concurrent Mode ready. So, excitedly, I've tried out the new useMutableSource in [email protected] in place of my current approach. It works pretty great! The useMutableSource hook takes care of requirements 1-3 out of the box only leaving requirement 4 to the implementer!

Right now, I'm looking for the following answers:

Question 1

How to correctly prevent re-rendering after store change if new snapshot is shallowly equal?

You can see my current approach below, but I don't think it's correct wrt Concurrent Mode.

UPDATE: I have updated the gist and code sandbox with a correct (?) implementation of selective subscription now. No more mutating refs.

Question 2

Is this type of implementation fully Concurrent Mode compatible?

I tried running this simple store implementation through the https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-mode test suite and the following checks fail:

✕ check 7: proper branching with transition
✕ check 9: no tearing with auto increment

My concern is - does that mean useMutableSource still has some tearing issues in some cases? (Note: I haven't looked into the the implementation of the test suite closely).

UPDATE Anyone know how to convert this super simple store into a fully React compliant store? E.g. I don't mind if it's turned into a useState/useReducer as long as it can be used in a similar manner?

Code

I've put the code used in my exercise here: https://codesandbox.io/s/flamboyant-keller-m9unj

• App.js - a demo app for exploring the 4 requirements above
• createStoreModern.js - simpler store used in useMutableSource implementaion
• useStoreModern.js - implementation of store hooks using useMutableSource
• createStoreLegacy.js - more complex store that allows order specified subscriptions
• useStoreLegacy.js - implementation of hooks without useMutableSource

I'm also including the code for the modern implementation directly below

createStoreModern.js

export function createStore(state, actions) {
  let listeners = []

  function get() {
    return state
  }

  function set(next) {
    state = { ...state, ...next }
    listeners.forEach(l => l())
  }

  function dispatch(type, payload) {
    actions[type]({ get, set }, payload)
  }

  function subscribe(f) {
    listeners.push(f)
    return () => {
      listeners = listeners.filter(ff => f !== ff)
    }
  }

  return { get, set, dispatch, subscribe }
}

useStoreModern.js

/*

This is a "modern" version of the store hooks, that could be used in the next version of tiny-atom
once useMutableSource lands in React. It handles our 4 requirements with ease:

1. Re-render at most once per request animation frame
this version does not render once per animation frame, but useMutableSource does batch changes within the same tick

2. Do not re-render children twice (if naively implemented – it happens once from parent, once from subscription)
useMutableSource takes care of that

3. Re-render parents first to avoid "zombie children"
useMutableSource takes care of that

4. Only re-render if selected slice of state changed
this is something we have to handle and I'm not sure about the correct way to do that

*/

import React, { useState, createContext, createMutableSource, useContext, useCallback, useMutableSource } from 'react'
import { createStore } from './createStore'
import { areEqual } from './areEqual'

const MutableSourceContext = createContext()
const MutableSourceProvider = MutableSourceContext.Provider

export function useStore(initialState, actions, deps = []) {
  const [store] = useState(() => createStore(initialState, actions))
  const [mutableSource] = useState(() => createMutableSource(store, () => store.get()))
  const [Provider] = useState(() => ({ children }) => (
    <MutableSourceProvider value={mutableSource}>{children}</MutableSourceProvider>
  ))
  return { Provider }
}

export function useSelector(selectorFn = x => x, deps = [], name = 'Unknown') {
  const mutableSource = useContext(MutableSourceContext)
  const selector = useCallback(selectorFn, deps)

  const getSnapshot = useCallback(
    memoize(store => selector(store.get())),
    [selector]
  )

  const subscribe = useCallback(
    (store, callback) => {
      let prevSnapshot
      return store.subscribe(() => {
        const nextSnapshot = getSnapshot(store)
        if (prevSnapshot !== nextSnapshot) {
          prevSnapshot = nextSnapshot
          callback()
        }
      })
    },
    [getSnapshot]
  )

  return useMutableSource(mutableSource, getSnapshot, subscribe)
}

export function useDispatch() {
  const { dispatch } = useStoreInstance()
  return dispatch
}

export function useStoreInstance() {
  return useContext(MutableSourceContext)._source
}

function memoize(fn) {
  let prev
  return (...args) => {
    const next = fn(...args)
    if (!areEqual(prev, next)) {
      prev = next
    }
    return prev
  }
}

const getSnapshot = useCallback(store => selector(store.get()), [selector])

getSnapshot must return an immutable object. I suspect what you return can be an object that is mutated later.
If you return an immutable object, then you would get the benefit of no re-rendering.

it's important to be able to not re-render if selected snapshot is shallowly the same - but the question is how?

If your getSnapshot returns an immutable object, but you still want to avoid re-renders if it's shallowly equal, you would need scoped subscriptions.
Check out this comment and the snippet linked in the comment. (It's obsolete though, because in my case I don't need shallow equal checks.)

https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-mode

Check 7 is not possible to pass with an external store. So, that's expected. Note, that doesn't mean it's CM-unsafe. It just means it can't get benefits from CM. (I need to add some notes there, but then I also need to explain state branching...)

Check 9 is an intentional test to distinguish useSubscription and useMutableSource. It's expected to pass with useMutableSource, but it's failing too on my end with 0.0.0-experimental-8b155d261.

General notes about the tool: As it tries to reproduce and test tearing (except for check 7) from outside React (except for check 9), it's very tricky (= adding long wait in render). These tests are not 100% accurate. There can be other cases for tearing, and there can be other issues to be CM-safe. Specifically, check 7 is not about CM-safe, but about benefits from CM.

@markerikson In case you didn't notice: I already solved the stale props issue (in a sense) in react-redux v7 reduxjs/react-redux#1505.
It's by running selectors only in renders, never in callbacks. I don't think we would take it seriously to merge it because it comes with the performance cost. What's important in that PR is that we have a spec for stale props, which passes with the change.

Right, that's basically what v6 did.

Thanks for the various pointers! Didn’t realise how much discussion on this already took place. Caught up with: reactjs/rfcs#147 And read the handy stale props gist: https://gist.github.com/markerikson/faac6ae4aca7b82a058e13216a7888ec Will try to use the “scoped subscriptions” in my implementation above. Will also have a think about the stale props issue. @dai-shi Would appreciate if you expanded a bit on the check’s 7 “CM benefits”. What exactly do you miss out on with “an external store” and how come there are several libs in that repo that pass that check?

…

On Mon, 16 Mar 2020 at 00:36, Mark Erikson ***@***.***> wrote: ***@***.**** commented on this gist. ------------------------------ Right, that's basically what v6 did. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://gist.github.com/26cb3f998899b525dbdfd85f7f3c7978#gistcomment-3214066>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACPGWDEJAVINXUE37W6UH3RHVYCNANCNFSM4LLH2VOQ> .

Would appreciate if you expanded a bit on the check’s 7 “CM benefits”.

There's another long thread: reduxjs/react-redux#1351

reduxjs/react-redux#1351 (comment)

yeah, the issue you're describing with "lack of state branching" is something Dan was specifically pinpointing.

Check out the official docs about branching and rebasing. Links in this tweet: https://twitter.com/dai_shi/status/1236865863476580352

What exactly do you miss out on with “an external store”

In short, you would see state changes in startTransition, while we want to show the stale state in the transition.

how come there are several libs in that repo that pass that check?

They use useState/useReducer underneath, which is the only way I know to support "state branching."

---

(Sorry for short responses. Can expand more if you need.)

Thanks for further clarifications. I've now updated the code above to not use a mutated useRef and instead do a proper scoped subscription.

Now I'm just wondering if it's possible to turn this store into a startTransition compatible version. It's not clear to me yet if its a limitation of useMutableSource or if it rather depends on how the underlying state is stored/evolved.

In other words - what I'm after is a global context (for storing things like user object, fetched data, routing info, etc.) that can be selectively subscribed to (like use-context-selector), but that's fully Concurrent compatible. Has anyone managed to solve that yet? :)

memoize(store => selector(store.get())),

Looks like you return a stable immutable result with getSnapshot. In this case, you don't need scoped subscriptions, because useMutableSource bails out if the snapshot is ref equal. Whether scoped subscriptions may improve the performance is unknown.

a global context, but that's fully Concurrent compatible.

I think it's possible. That's actually what I did: dai-shi/react-tracked#43
It doesn't pass the automated test with the tool, but works as I expect when I try it by hand.