Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for React Concurrent mode #228

Open
luisherranz opened this issue Jan 24, 2021 · 2 comments
Open

Support for React Concurrent mode #228

luisherranz opened this issue Jan 24, 2021 · 2 comments

Comments

@luisherranz
Copy link

Is your feature request related to a problem? Please describe.

The upcoming React Concurrent mode will be capable of working on different branches at the same time and then discard or merge them depending on the situation. React libraries like react-easy-state need to prepare for renders that will start but will never make it to the DOM.

Currently, the reactions (observe) created internally by view are disposed (unobserve) on the unmount phase (the return function of useEffect). That will no longer be possible in React Concurrent because some renders will not make it to the mount/unmount phase.

This means we need to find an alternative way to dispose the reactions of the components that are discarded or we will have a memory leak.

Describe alternatives you've considered or seen elsewhere

I have been studying how the people of MobX has solved this.

They added support for this in the v2 of mobx-react-lite. These are the changes they did to their useObserver in the v2 version: mobxjs/mobx-react-lite@v1.5.2...v2.0.0#diff-33c284d6bf49fd504d1250e7090d24ab73dae525efcc6376363ec89722a0df32

They added two fixes:

  1. Do not trigger rerenders for components that are not mounted.

    I have already done a PR to do that in Fix React's StrictMode warning: unmounted component rerender #227, but that doesn't solve the memory leak.

  2. Create an ad-hoc garbage collector for the reactions of unmounted components:

A couple of months ago they started using the new FinalizationRegistry API to get a callback when a component is garbage collected by the JS engine and also dispose the corresponding reaction, but they still have the ad-hoc garbage collector as fallback for old browsers.

Describe the solution you'd like

I think the solution of MobX is unnecessarily complicated. I guess they must need to do so because of the way MobX stores reactions internally, but I don't think using a manual garbage collection is the correct approach for react-easy-state.

What I would do is to create a new type of weak reactions, that are stored in a WeakMap inside the observable (instead of the current Map) and are garbage collected automatically by the JS engine when the reference to the reaction is lost.

This would require the exposure of a new weakObserve API from @nx-js/observer-util that will save those reactions in a WeakMap here: https://github.com/nx-js/observer-util/blob/master/src/store.js#L1-L7

const connectionStore = new WeakMap();
const connectionWeakStore = new WeakMap();

export function storeObservable(obj) {
  // save normal observe() reactions
  connectionStore.set(obj, new Map());
  // save weakObserve() reactions
  connectionWeakStore.set(obj, new WeakMap());
}

Weak reactions would be declared with the new API and are only held in memory while the reference is stored somewhere:

observe(() => {
  // I am going to run forever.
});

const reaction = weakObserve(() => {
  // I am going to run only while `reaction` is referenced somewhere.
});

The reference to reaction will be held internally by the components using view because they need it for the unmount phase. But if a component is garbage collected, the reference is lost and the reaction will go away with it.


I will be glad to work on a PR to do this but I would like to know @solkimicreb's opinion first to make sure that this is the right approach 🙂

@Agamennon
Copy link

I was considering RES, for a project but i was worried about future proofing the libraries, i understand that react concurrent mode will inevitably come, just out of curiosity @luisherranz did you solved this issue forking @solkimicreb nx-js/observer-util, is there news on this issue? thanks!

@luisherranz
Copy link
Author

No news yet. I want to know @solkimicreb's opinion, although he is really busy right now so it may take a while 🙂

But we still have time until React Concurrent is finally released so don't worry: facebook/react#13206

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants