Support concurrent features React / React 18

[mweststrate]

Support concurrent mode React. Primary open issue, don't leak reactions for component instances that are never committed.

A solution for that is described in: https://github.com/mobxjs/mobx-react-lite/issues/53

But let's first wait until concurrent mode is a bit further; as there are no docs yet etc, introducing a workaround in the code base seems to be premature at this point (11-feb-2019)

[imjordanxd]

Hi, @seivan. I'm particularly interested Mobx's alignment with this repo. Mobx is blocking us from upgrading to React 18

[kubk]

@imjordanxd It might be useful for you: https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-rendering/pull/63

The reason why it's rejected is that implementation doesn't use a common reducer (which is required for this repo but isn't required for React)

[urugator]

Just to let you know, I am working on some improvements. I've managed to rewrite observer for functional components, so it uses useExternalSyncStore. There aren't many tests, but it seems to be working fine. We have to do some things that I am not sure are completely cool, like calling onStoreChange during subscribe, but as I said it seems to be working atm. What remains is concurrency support for class components, which I would like to include in this change as well. One thing I am thinking about right know is, that we won't be able to use FinalizationRegistry here, because the only thing we can register is this, but since it's accessible by user, I think it's too easy to leak it outside the component.

[urugator]

The effort is here https://github.com/mobxjs/mobx/pull/3590, should be more or less complete.

[ericmasiello]

Fwiw, I did some experiments with mobx at this repo: https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-rendering

Regular, unpatched observer failed 4 tests and suffered some tearing issues for a couple of more basic tests.

I then patched observer and replaced forceUpdate with:

const reactionTrackingRef = React.useRef(null);

let forceUpdate;

if (!reactionTrackingRef.current) {

    const newReaction = new Reaction(observerComponentNameFor(baseComponentName), function () {

        if (trackingData.mounted) {

            version = version + 1;

            forceUpdate();

        }

        else {

             trackingData.changedBeforeMount = true

        }

    });

    const trackingData = addReactionToTrack(

        reactionTrackingRef,

        newReaction,

        objectRetainedByReact

   );

}

useSyncExternalStore(

    useCallback((onStoreChange) => {

        forceUpdate = () => {

            onStoreChange();

        }

    }, []),

    () => version

);

This only failed the two level 3 tests (5 & 6), which seems to be more in line with the current level support of some other major state libraries (see this table).

I did some quick manual tests and did not detect any extra renders.

@k-ode do you mind sharing the code you used to test MobX against these tests? I recall there is some scaffolding required

[k-ode]

@ericmasiello Sure, here it is (most likely out of date though).

src/mobx/index.js

import React, { useCallback } from 'react';
import { observable, runInAction } from 'mobx';
import { observer } from 'mobx-react';

import {
  reducer,
  initialState,
  selectCount,
  incrementAction,
  doubleAction,
  createApp,
} from '../common';

const state = observable(initialState);

const useCount = () => selectCount(state);

const useIncrement = () => useCallback(() => {
  const newState = reducer(state, incrementAction);
  runInAction(() => {
    Object.keys(newState).forEach((key) => {
      state[key] = newState[key];
    });
  });
}, []);

const useDouble = () => useCallback(() => {
  const newState = reducer(state, doubleAction);
  runInAction(() => {
    Object.keys(newState).forEach((key) => {
      state[key] = newState[key];
    });
  });
}, []);

export default createApp(useCount, useIncrement, useDouble, React.Fragment, observer);

src/common.js

diff --git a/src/common.js b/src/common.js
index 017acbf..ffe60a3 100644
--- a/src/common.js
+++ b/src/common.js
@@ -80,7 +80,7 @@ export const createApp = (
     return <div className="count">{count}</div>;
   });

-  const Main = () => {
+  const Main = componentWrapper(() => {
     const [isPending, startTransition] = useTransition();
     const [mode, setMode] = useState(null);
     const transitionHide = () => {
@@ -142,7 +142,7 @@ export const createApp = (
         <div id="mainCount" className="count">{mode === 'deferred' ? deferredCount : count}</div>
       </div>
     );
-  };
+  });

   const App = () => (
     <Root>

[penx]

Slightly related as there's a lot of useSyncExternalStore discussion here, I wonder if this would be a useful export from mobx-react/mobx-react-lite, so that observables can be used in hooks outside of an observer?

export const useSelector = <T>(selector: () => T): T => {
 return useSyncExternalStore(autorun, selector);
}

Discussion here https://github.com/mobxjs/mobx/discussions/3589

[mweststrate]

closing as landed.