React: [Concurrent] Safely disposing uncommitted objects

[FredyC]

How to safely keep a reference to uncommitted objects and dispose of them on unmount?

For a MobX world, we are trying to prepare for the Concurrent mode. In short, there is a Reaction object being created to track for observables and it is stored within useRef.

The major problem is, that we can't just useEffect to create it in a safe way later. We need it to start tracking the observables on a first render otherwise we might miss some updates and cause inconsistent behavior.

We do have a semi-working solution, basically, a custom made garbage collector based on setTimeout. However, it's unreliable as it can accidentally dispose of Reactions that are actually being used but weren't committed yet.

Would love to hear we are overlooking some obvious solution there.

[gaearon]

@mweststrate

[FredyC]

No need to ping him, he knows about it.

Either way, I wanted to ask for a more generic solution, not only related to a MobX. I can imagine similar problems will be happening all over the place once people start preparing for Concurrent. It feels like that React is not giving enough tools to tackle the problem gracefully.

[gaearon]

I’m tagging him precisely because “he knows about it” and because we’ve had conversations in the past about it. :-)

Given that he knows constraints of both I thought his input could be valuable.

As for your question — the design tradeoff we’re going for is that we don’t have a cleanup phase for renders that never committed. Rendering isn’t supposed to “allocate” resources that have to be cleaned up later. Instead we rely on the garbage collector. Resources that have to be disposed are only safe to allocate after the first commit.

I don’t know for certain if tradeoffs chosen by MobX are compatible or not with this approach, as I don’t know enough about MobX.

Can you explain what “reactions” are and why GC isn’t sufficient for them? Or why creating them can’t wait until after the first commit?

[FredyC]

Can you explain what “reactions” are and why GC isn’t sufficient for them? Or why creating them can’t wait until after the first commit?

Well, I don't know exact internals how it works, it's very entangled there, but my assumption is that Reaction kinda attaches itself to observable variables present in the callback function. So as long as those variables exist, the Reaction won't be GCed. Those variables can definitely exist outside the scope of the component, that's kinda point of the MobX.

I think we have managed to figure out some way around it. Custom GC :) Basically, we globally keep the list of created reactions (in WeakMap) along with a time of their creation. If Reaction gets committed, all is fine and it gets removed from the list. A global timer takes care of cleaning up those that weren't committed in a given time.

I am wondering if you have some idea for a good "timeout" period? I mean time from render to commit. For now, we are thinking like 1 second. I assume it should be a long enough period even on a really slow computer. Is it bad idea to rely on that?

[FredyC]

Or why creating them can’t wait until after the first commit?

@RoystonS wrote a good explanation of that. We could end up in very ugly situations ...

[urugator]

Can you explain what “reactions”

Mobx automatically manages subscriptions based on usage.
You define a function (reaction), which can access observable objects.
When the function is invoked, the observables accessed inside the function are added as dependencies to the function (reaction).
When you modify an observable, the function is automatically invoked again, resubscribing and optionally running a side effect.
We wrap render inside such function, so that if you access an observable in render, it creates a subscription:

const disposer = reaction(
  () => {    
    // accesses observables, creating subscriptions
    render()
  }, 
  () => {
    // a side effect to invalidate the component
    setState() // or forceUpdate() 
  }
)

why creating them can’t wait until after the first commit

The subscriptions are determined by rendering logic, so we would have to re-run the rendering logic in effect (double render).

why GC isn’t sufficient for them

While reactions come and go (together with react's component instances), the observable objects possibly stay for the whole app lifetime and they hold references to reactions (preventing them from being GCed).

[RoystonS]

Basically: we're trying to avoid double-rendering _all_ MobX-observer components. The only time we actually _need_ to double-render is if StrictMode/ConcurrentMode began rendering our component but then never got to the commit phase with us, or if something we're observing changed between initial render and commit. Taking a double-render hit on _every_ component to cope with those (hopefully edge) cases is something we'd _like_ to avoid.

[mweststrate]

@gaearon @urugator's description is pretty accurate. Whether it should be fixed through a React life-cycleish thing or by doing some additional administration on MobX side (timer + custom GC) have both it's merits. So let's leave that aside for now, and just focus on the use case:

Basic concepts

But the reason why immediate subscription is important to MobX (as opposed to a lifecycle hook that happens after render, such as useEffect) is because observables can be in 'hot' and 'cold' states (in RxJS terms). Suppose we have:

1. A collection of todo items
2. An expression (we call this a computed) that computes the amount of unfinished computed items
3. A component that renders the amount of unfinished computed items.

Now, the expression computed can be read in two different modes:

1. If the computed is cold, that is, no one is subscribed to it's result; it evaluates as a normal expression, returning and forgetting its result
2. If the computed is hot, it will start observing its own dependencies (the todo items), and caching the response as long as the todo item set is unaffected and the output of the computed expression is unchanged.

MobX without concurrent mode

With that I mind, how observer was working so far is as follows:

1. During the first (and any subsequent) render, MobX creates a so called reaction object which keeps track of all the dependencies being read during the render and subscribes to them. This caused the computed to become hot immediately, and a subscription to the computed to be set up. The computed in turn subscribes to the todos it used (and any other observable used). So a dependency graph forms.
2. Since there is an immediate subscription, it means that the reaction will no longer be GC-ed as long as the computed exists. Until the subscription is disposed. This is done automatically done during componentWillUnmount. This cause the reaction to release its subscriptions. And in turn, if nobody else depends on the 'computed' value, that will also release its subscriptions to the todos. Reaction objects can now be GC-ed.

So what is the problem?

In concurrent mode, phase 2, disposing the reaction, might no longer happen, as an initial render can happen without matching "unmount" phase. (For just suspense promise throwing this seems avoidable by using finally)

Possible solutions

Solution 1: Having an explicit hook that is called for never committed objects (e.g. useOnCancelledComponent(() => reaction.dispose())

Solution 2: Have global state that records all created reaction objects, and remove reactions belonging to committed components from that collection. On a set interval dispose any non-committed reactions. This is the current (WIP) work around, that requires some magic numbers, such as what is a good grace period? Also needs special handling for reactions that are cleaned up, but actually get committed after the grace period!

Solution 3: Only subscribe after the component has been committed. This roughly looks like;

1. During the first render, read the computed value cold, meaning it will be untracked
2. After commit (using useEffect) render another time, this time with proper tracking. This serves two goals:
   
   
   
   1. The dependencies can be established
   
   
   2. Any updates to the computed between first rendering and commit will be picked up. (I can imagine this is a generic problem with subscribing to event emitters as part of useEffect, updates might have been missed?)
   
   

The downsides:

1. Every component will render twice!
2. The computed will always compute twice, as there is first a 'cold', which won't cache, and then a 'hot' read. Since in complicated applications there might be entire trees of depending computable values be needed, this problem might be worse than it sounds; as the dependencies of a computed value also go 'cold' if the computed value itself goes cold.

Hope that explains the problem succinctly! If not, some pictures will be added :).

[gaearon]

Does MobX have bigger problems in Concurrent Mode? How valuable is it to fix this in isolation without addressing e.g. rebasing?

[mweststrate]

I think the later problem can be largely avoided by making a clear
distinction between component own state (use useState instead of MobX) and
state that lives outside the component tree (external domain state).

If I am not mistaken, my intuition is that rebasing only makes sense for
'volatile ui state' such as the state of pending requests, switching tabs /
pages, charts etc. But state that is domain specific (e.g. a collection of
todo's), typically doesn't need rebasing as that would lead to weird
behavior for the end-user?

On Wed, Apr 10, 2019 at 4:36 PM Dan Abramov [e-mail]
wrote:

Does MobX have bigger problems in Concurrent Mode? How valuable is it to
fix this in isolation without addressing e.g. rebasing?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/15317#issuecomment-481716405,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhOMREb3lbrEL4q9sccIXfNj5f5vfks5vffcHgaJpZM4ccUAP
.

[NE-SmallTown]

Any picture? Thanks~ :)

[radex]

We have a very similar issue to MobX's with 🍉 WatermelonDB, but even more tricky because of asynchronicity.

When connecting the data source to components, we subscribe to the Observable. This has to happen directly in render, because:

• if we subscribe in an effect/on did mount, we'll ALWAYS render twice, and the first render will be wasted, even if we have the data ready to render and we can skip a bunch of unnecessary work.
• it wouldn't be appropriate for an useObservable hook to return null on initial render, because it's not always clear what the component developer is supposed to do with it. For example, if an observable value is necessary to fetch another observable value, the developer would have to make a whole bunch of extra logic just to get to the return null (since we can't just bail early and render null with hooks - like we can today with a withObservables HOC).
• it would be appropriate for useObservable to throw a Promise and suspend, however, we risk showing stale data (which can lead to bugs or ugly glitches) if we treat the data source like an async promise, not a reactive Observable — it could be that between the time the initial data is fetched, and component re-rendered, the value of the observable has changed again. But it's not possible to know that on the second render if we don't stay subscribed to it. Which means we have to subscribe before we throw a Promise — and have to deal with cleaning up the subscription if the component never ends up being rendered

[CaptainN]

We have a similar situation in Meteor, where we set up a computation and its observers the first time it's run. If we use useEffect we have to either wait for useEffect to do that, causing a render of no content, or use useLayoutEffect without touching the dom, which would violate the suggestions and complicate the API for our users and still render twice if somewhat synchronously, or render twice with useEffect - once without setting up observers which is hacky (or doing a full create/dispose lifecycle) and then always at least one additional time after useEffect (this was janky in testing).

What I ended up doing is something based on setTimeout, but it takes a lot of juggling (we also have some backward compat code which complicates our implementation. It has to set up the computation and a timeout, then in useEffect it has to look for a few things; Is the computation still alive (did setTimeout trigger dispose)? If not rebuild it, and re-render (back to 2 renders). Did we get a reactive change between render and useEffect? If so, re-run the computation, and re-render (this is fine, because it should anyway).

I originally set the timeout to 50ms, assuming commit would happen relatively quickly, but in testing in concurrent mode 50ms missed almost all commits. It often took longer than 500ms to commit. I've now set the timeout to 1000ms. I also saw that there are a lot of discarded renders for every commit, between 2-4 to 1. (I have no idea how to write tests for all this, this is based on some counter/console logging.)

Since there is a lengthy gap between render and commit, I had to decide what to do for reactive changes that happened in renders which will never be committed. I could either run the user's reactive function for every change, knowing it might be run by 4 different handlers without doing anything, or block reactive updates until the component was committed. I chose the latter.

I tried a workaround using useLayoutEffect. I incorrectly assumed it would run in the same event loop as render. If it had, the code for juggling when and if to build/rebuild/re-run/re-render could all be greatly simplified, and a timeout of 0ms or 1ms would work just fine. In Strict Mode, this worked beautifully (and greatly simplified our implementation). In practice, in concurrent mode, useEffect and useLayoutEffect performed no differently from each other.

BTW, one additional thing we found challenging was trying to leverage a deps compare. We had at one point copied some parts of react out of core to manage that, before just switching to useMemo to use its deps compare without it's memoization. I understand that may lead some some additional re-creations, our old HOC used to do that anyway (and it does that now if a user supplies no deps to our useTracker hook), so I went with useMemo just to reduce our overhead - but it'd be nice if the deps compare functions were somehow available to use.

[CaptainN]

A quick additional observation - one of the things I have in place is a check to make sure that if the timeout period elapsed (currently 1 second), it will make sure to restart our computation when/if the render is eventually committed in useEffect. This has almost never triggered outside of concurrent mode (and it rarely triggers inside concurrent mode at 1second). It did trigger today during a particularly janky local dev reload. All of this would be so much simpler if there was a way to reliably clean up after discarded renders without resorting to timeout based GC hacks.

[aleclarson]

Using dual-phased subscriptions (phase 1: collect observed object/key pairs and their current values, phase 2: subscribe to them), you can avoid any timer-based GC and postpone subscribing until React commits. In phase 2, be sure to check for changed values before subscribing, so you can know if an immediate re-render is required. This is what I did with the Auto class in wana. Of course, this only applies to observable objects. Streams and event emitters, not so much.

[CaptainN]

We'd have to modify a bunch of stuff in Meteor core to convert that to a dual-phase subscription model like that (although we can fake it by just setting up the subscription, running the computation, then disposing of it immediately, to restart it again in useEffect). An additional problem is that the return data from a Meteor computation is not immutable, and can be any type of data (with possibly very deep structures), so a deep compare would be necessary to make sure data hasn't changed, or we'd have to render twice (same problem as before).

[aleclarson]

We'd have to modify a bunch of stuff in Meteor core to convert that to a dual-phase subscription model like that

The issue with Meteor is that its tracking system doesn't associate "observable dependencies" with state. In other words, the Tracker.Dependency instance is disconnected from what it represents. Although, you could get around that by updating a nonce property on the dependency whenever it emits a change event.

(although we can fake it by just setting up the subscription, running the computation, then disposing of it immediately, to restart it again in useEffect)

Wouldn't you miss changes between render and commit if you did it that way?

An additional problem is that the return data from a Meteor computation is not immutable, and can be any type of data (with possibly very deep structures), so a deep compare would be necessary to make sure data hasn't changed, or we'd have to render twice (same problem as before).

Yep, that's the problem with not using observable proxies that track every property access.

[aleclarson]

Another good use case for useCancelledEffect is being able to lazily allocate shared state (which is stored in a global cache or in context) during render, without the risk of leaks or the need to implement a timer-based GC.

[CaptainN]

Wouldn't you miss changes between render and commit if you did it that way?

Yes. It's actually that way now even with the timer. If an update comes in before the render is committed (which takes about a half a second in my testing), I had to make a choice - run the user's reactive function for every uncommitted computation, or don't run any of them until the render is committed (I detect if an update happened, but don't run user code until later) - I chose the latter. In the example I gave in the other comment, I'm saying we could simply always rerun the computation and user function again in useEffect - but then we have to always eat a second render, and all the rest.

Another good use case for useCancelledEffect is being able to lazily allocate shared state (which is stored in a global cache or in context) during render, without the risk of leaks or the need to implement a timer-based GC.

I had thought it would be useful if we could get some kind of deterministic ID for whatever the current leaf is, which is consistent for all renders of current component, even those that would not be committed. Then I could use my own global store. I think someone explained that couldn't happen though, for various reasons. (Can react-cache be used for this? I'm not clear on what that is useful for.) I could offload the naming/ID responsibility to the user, but that seem onerous, and I'm not sure how I'd be able to detect collisions - though, would I have to? 🤔

[aleclarson]

This problem also exists without Concurrent mode enabled, because no-op state changes can trigger a render that bails out before effects are ever called. Source

[kitten]

We've had to find a solution to this for urql. It's a similar problem to what MobX has in that we're using streams that start a GraphQL operation when subscribed to which then yields either synchronous or asynchronous results (cached or fetching).

The solution to this can be found here: https://github.com/kitten/react-wonka/blob/master/src/index.ts
We're essentially dealing with 1) synchronous subscriptions since they may immediately return a result, so they can't be started in useEffect only, 2) cleanups on interrupted or double renders, 3) updates to values in useEffect.

I'll leave some simplified / generic code here in case someone else has to implement it from scratch, since we haven't found a generic solution yet that could be shared (like an alternative to use-subscription for instance)

Interruptible Subscriptions implementation walkthrough

On mount we start the subscription if it hasn't started yet:
_(note: most of this is pseudo code since the full implementation is linked above)_

const useObservable = observable => {
  const subscription = useRef({
    value: init,
    onValue: value => {
      subscription.current.value = value;
    },
    teardown: null,
    task: null,
  });

  if (subscription.current.teardown === null) {
    // Suppose we have a subscription that needs to be started somehow, with a teardown:
    subscription.current.teardown = observable.subscribe(value => {
      subscription.current.onValue(value)
    });
  }

  return subscription.value;
};

Then we schedule an immedite teardown/cancellation of the subscription using scheduler (this way we get really predictable and cooperative timing):

import {
  unstable_scheduleCallback,
  unstable_cancelCallback,
  unstable_getCurrentPriorityLevel,
} from 'scheduler';

const useObservable = observable => {
  // ...
  if (subscription.current.teardown === null) {
    subscription.current.teardown = observable.subscribe(value => {
      subscription.current.onValue(value)
    });

    subscription.task = unstable_scheduleCallback(
      unstable_getCurrentPriorityLevel(),
      subscription.teardown
    );
  }

  return subscription.value;
};

On interrupted rendering we won't have any effects run (useEffect and useLayoutEffect) but on a normal render we don't want to cancel the subscription by calling the teardown. We can use useLayoutEffect to cancel the teardown (unstable_cancelCallback) since that has synchronous timing (after mount during the commit phase)

const useObservable = observable => {
  // ...
  if (subscription.current.teardown === null) {
    subscription.current.teardown = observable.subscribe(value => {
      subscription.current.onValue(value)
    });

    subscription.task = unstable_scheduleCallback(
      unstable_getCurrentPriorityLevel(),
      subscription.teardown
    );
  }

  useLayoutEffect(() => {
    // Cancel the scheduled teardown
    if (subscription.current.task !== null) {
      unstable_cancelCallback(subscription.current.task);
    }

    // We also add the teardown for unmounting
    return () => {
      if (subscription.current.teardown !== null) {
        subscription.current.teardown();
      }
    };
  }, []);

  return subscription.value;
};

Lastly we'd like a normal useEffect to take over for normal cooperative effect scheduling. In the real implementation we also send updates back to a subject which affects the observable, so we do that there as well. For simplification/generalisation purposes that's excluded here though.

const useObservable = observable => {
  // ...

  // We add an updater that causes React to rerender
  // There's of course different ways to do this
  const [, setValue] = useReducer((x, value)) => {
    subscription.current.value = value;
    return x + 1;
  }, 0);

  if (subscription.current.teardown === null) {
    // ...
  }

  useLayoutEffect(() => {
    // ...
  }, []);

  useEffect(() => {
    // In useEffect we "adopt" the subscription by swapping out the
    // onValue callback with the setValue call from useReducer above
    subscription.current.onValue = setValue;

    // If we've cancelled the subscription due to a very long interrupt,
    // we restart it here, which may give us a synchronous update again,
    // ensuring that we don't miss any changes to our value
    if (subscription.current.teardown === null) {
      subscription.teardown = observable.subscribe(value => {
        subscription.onValue(value)
      });
    }

    // We'd typically also change dependencies and update a Subject,
    // but that's missing in this example 
  }, []);

  return subscription.value;
};

Lastly we need to update this to work on server-side rendering.
For that we swap out useLayoutEffect with a useIsomorphicEffect

const isServerSide = typeof window === 'undefined';
const useIsomorphicEffect = !isServerSide ? useLayoutEffect : useEffect;

And add some code that immediately cancels the subscription on the server-side:

const useObservable = observable => {
  // ...
  if (subscription.current.teardown === null) {
    subscription.teardown = observable.subscribe(value => {
      subscription.onValue(value)
    });

    if (isServerSide) {
      // On SSR we immediately cancel the subscription so we're only using
      // synchronous initial values from it
      subscription.current.teardown();
    } else {
      subscription.current.task = unstable_scheduleCallback(
        unstable_getCurrentPriorityLevel(),
        subscription.current.teardown
      );
    }
  }
  // ...
};

Afterwards we added some code that handles our updates to an input value.

tl;dr We start the subscription on mount synchronously, schedule the teardown using scheduler, cancel the scheduled teardown in useLayoutEffect (since that tells us that effects will run and we're not in an interrupted render), let a useEffect adopt the subscription and switch an onValue updater over to a useReducer update.

If note is that this only works as well as it does for us because we're already handling starting operations only once if necessary and avoiding duplicates, so in a lot of cases our subscriptions are already idempotent. I can totally see that this isn't the case for everyone, so a lot of subscriptions with heavy computations won't be as "safe" to run twice in rapid succession.

This is a little inconvenient to say the least. I'd love if either use-subscription or a built-in primitive could introduce a concept of observing changing values that starts a subscription synchronously but also ensures it's cleaned up properly on interrupts or unmounts. 🤔

[mweststrate]

@FredyC At a quick glance, the above might be cleaner than the current approach in lite 2. Thoughts? (Probably better to discuss further in the repo, just wanted to cc you in!)

[dai-shi]

It's really interesting.

we haven't found a generic solution yet that could be shared (like an alternative to use-subscription for instance)

Let me give a quick trial in TS:

import { useState, useRef, useLayoutEffect } from 'react';
import {
  unstable_scheduleCallback as scheduleCallback,
  unstable_cancelCallback as cancelCallback,
  unstable_getCurrentPriorityLevel as getCurrentPriorityLevel,
} from 'scheduler';

type Subscription<Value> = {
  currentValue: Value; // synchronous value
  unsubscribe: () => void;
};
type Subscribe<Value> = (callback: (nextValue: Value) => void) => Subscription<Value>;

export const useSubscription = <Value>(subscribe: Subscribe<Value extends undefined ? never : Value>): Value => {
  if (subscribe !== useRef(subscribe).current) {
    throw new Error('no support for changing `subscribe` in this PoC');
  }

  const [value, setValue] = useState<Value | undefined>();

  const subscription = useRef<Subscription<Value>>();
  const cleanup = useRef<ReturnType<typeof scheduleCallback>>();
  if (!subscription.current) {
    subscription.current = subscribe(setValue);
    cleanup.current = scheduleCallback(getCurrentPriorityLevel(), subscription.current.unsubscribe);
  }

  useLayoutEffect(() => {
    if (cleanup.current) cancelCallback(cleanup.current);
    return subscription.current?.unsubscribe;
  }, []);

  return value !== undefined ? value : subscription.current.currentValue;
};

[CaptainN]

@dai-shi How does that scheduleCallback function work? Does it use a timeout?

[dai-shi]

@CaptainN I re-organized what @kitten described in a general way. I haven't tested it (at least yet).
Having said that, my understanding is that
1) scheduleCallback registers a callback into a queue,
2) useLayoutEffect runs in a higher priority (or the same but somehow before it?),
3) thus cancels the registered callback in the queue before executing.

setTimeout would be used in scheduler, but not like in a naive way.

@kitten @JoviDeCroock Did you figure out which priorities are used in render and layout effect? NormalPriority for the former and ImmediatePriority for the latter, I presume, but not confirmed.

---

(edit) On second thought, I'm a bit suspicious about this. Suppose, if one of render is really slow (like taking several seconds), React will likely to throw away that render result on updates, but it may(?) keep other components that are not committed yet and will commit them after a long time.

[CaptainN]

@dai-shi That's why I asked it was timeout based. In the Meteor hook we implemented, we just use a timeout. I found that a full 1000ms was needed in order for the subscription (computation in our case) to be preserved in some large percentage of cases (at least on my machine). We had to do a couple of other things though - we also will destroy the computation if an update comes in before the render is committed. And we also check in useEffect if we need to rebuild the computation.

There's a lot of overhead to implement this correctly, all of it is difficult, and there are dozens of bad examples out on the net masquerading as good suggestions, most of which will leak memory and resources, mostly silently. There really needs to be an easier properly supported solution to this common problem.

[FredyC]

I wonder is the take on this from the React team. Apparently, the use cases are pilling up and with Concurrent more getting more attention it would be silly to expect everyone implementing some hacky workarounds.

Just a wild idea, but what about Suspense? Can we, in theory, suspend rendering until any sort of "rendering setup" is ready? I am not really sure what would be the implications. I know just basics about the whole thing. Would it harm the performance?

@sebmarkbage Can you possibly give some thoughts here? Is this something the React simply won't support officially?

[CaptainN]

@FredyC Suspense is similarly unintuitive. It doesn't "suspend" execution, and then later commit. After you throw a promise, later on a fresh render runs again from scratch. But that doesn't give us the info we need anyway. Suspense is great for when our resource is not yet ready. But if our resource is ready (and it almost always is in Meteor) during the first render, concurrent mode will run it however many times (up to 4 in my testing), and then only commit one of them. But there's no reliable or easy way to determine which ones are tossed.

It'd be great if there were some sort of way to know that all /n/ renders are candidates for the same commit - that'd be ideal. We could run the effect once for the pool, and associate it with the committed render, but that's probably too much overhead or something to set up.

If only there was a "componentWillMount" hook. 🤔

[dai-shi]

While waiting for the official answer, which I think is "won't support", here's another suggestion. Coming from the basics, we make render function side effect free.

const useSubscription = (subscribe, initialValue) => {
  const [value, setValue] = useState(initialValue);
  useLayoutEffect(() => {
    const { currentValue, unsubscribe } = subscribe(setValue);
    setValue(currentValue);
    return unsubscribe;
  }, [subscribe]);
  return value;
};

One caveat as you see is that developers who use this hook has to provide initialValue.
But, end users won't see the initialValue rendered because it's the layout effect.

I'm pretty sure many of us already tried such an approach, but just an option to re-consider as it's the closest one to componentWillMount.

---

Yet another approach would be to store the current value in a global cache.

const cache = new WeakMap();

const useSubscription = (subscribe) => {
  if (!cache.has(subscribe)) {
    const { currentValue, unsubscribe } = subscribe(() => null /* noop */);
    cache.set(subscribe, currentValue);
    unsubscribe();
  }
  const [value, setValue] = useState(() => cache.get(subscribe));
  useEffect(() => {
    const { currentValue, unsubscribe } = subscribe(setValue);
    cache.set(subscribe, currentValue);
    setValue(currentValue);
    return unsubscribe;
  }, [subscribe]);
  return value;
};

This approach can be done with the official use-subscription, which might be appropriate.
Obviously, this pattern is not very nice, if we often need customized subscribe functions, like one-to-one to components.

[FredyC]

The useLayoutEffect even though it runs almost immediately, it's still runs after the first render happens. For MobX purposes, it's useless as we need to watch over that first render.

The global cache might be almost good only if there would be some reliable identity of the component _instance_. Especially the identity that would be the same over the Concurrent mode _retries_. We cannot expect users to pass that identity there. Generating something out of thin air and storing with useRef or useState is useless too as we end up with exactly the same problem of having multiple reactions and we won't know which one to dispose of.

[dai-shi]

@FredyC
For the MobX case, it would be ideal to work with dual-phased subscriptions.
It would be nice if MobX provides such capabilities. As I look at the mobx code, unfortunately not. (Now, I understand where your concerns are coming from.)

if there would be some reliable identity component instance

Totally right about that. I forgot that useCallback is also reset.

[kitten]

@dai-shi I mean, to get this out of the way, this issue is explicitly _not_ about "dual-phased subscriptions" (let's just say subscriptions via/in useEffect to keep things simple), so I do think we all know that that does work, but we'd like our subscriptions to be represented as values in React 1:1 and not _eventually_ match up 😅

[sebmarkbage]

Re: connecting to an external store.

There are a lot of implementation details of current systems. That might make upgrading hard but that doesn't say whether it's a good idea longer term. I think ultimately this problem bottoms out in reading a mutable value in a newly created component (already mounted components can use snapshots in their state). Depending on if your system also wants to support Suspense this might get conflated with Suspense, see below.

We're working on a new hook for mutable external values that deopts in scenarios where it would produce an inconsistent tree. By doing a full batched render in synchronous mode. It's "safe" and it does allow a tree to commit as one. It's unsatisfactory because it will deopt and produce unfortunate perf characteristics in certain scenarios.

It's related to this thread, because it aims to solve scenarios that I'm not sure has been considered yet. I believe that even if we had a clean up hook, you would still leave unsafe scenarios on the table or at the very least deopt in more cases.

In terms of a mutable store, the subscription fills the purpose of invalidating the render if it changes before commit. Otherwise the useSubscription pattern works. However, you also have to think about whether invalidating the render should also take any higher pri updates with it. It's not just about consistency but whether you merge multiple timelines. Additionally, if you always invalidate a
render for changes to the store you risk starving out. I think what you really want is to commit the work you've already done even if it's now stale, as long as that commit is consistent with itself.

That means we should continue rendering until we find a case that is impossible to make consistent and only deopt once we hit that. Because if you use it sparingly you don't have to deopt at all. However, you don't need a subscription to accomplish this because it's not the already rendered component that determines whether the tree is inconsistent, it's the next rendered component that we haven't gotten to yet.

Re: suspense.

I agree that suspense intuitively feels like you should be able to open connections during render. That's what our original demos showed. It seems right. However, there are a few caveats.

One is that even when React does its best to preserve already started work, it's surprisingly easy for rebases, especially long lived ones, to invalidate it. Therefore I believe you still need something like a timeout based cache to solve this regardless of what React does.

Additionally, there are other problems with this approach with regard to when invalidation of data that has already rendered should happen. The solution to that is keeping an object in state that represents the current lifetime like we showed at the latest React Conf. If you use that approach then you also solve the problem of when to invalidate opened requests. When you explicitly cancel the lifetime of the subtree, then you cancel any requests opened by that subtree.

Now, in theory we could do a traversal for most cases but it wouldn't be fool-proof and it would make React slower for everyone. To be convinced of this, we would have to see that the whole system overall that spawns from this actually works well, not just in theory, and that its convenience is worth making React slightly slower for everyone.

[FredyC]

I am probably misunderstanding something, but having a hook to deopt rendering won't work well for MobX case. It would mean to _slow down_ most of the components that work with observables on purpose, right? Or to be precise it would be same performance as without Concurrent mode. That sounds more like a foot gun and not a solution.

With the current solution based on timers, it's ugly, but there is only a risk memory leaks which is not that bad in comparison to deoptimizing rendering on purpose.

@sebmarkbage By any chance, the idea of having hook that provides _unique component instance identity_ does not sound useful? It should be the same over the rendering _attempts_. Ideally some sort of object so it can be utilized in WeakMap and we will be able to keep info about things that should happen only once.

[dai-shi]

I mean, to get this out of the way, this issue is explicitly not about "dual-phased subscriptions"...

Please forgive me, @kitten. I was wrong.
I just stumbled upon this issue while I was developing a new library.

I may misunderstand something either for suspense support. Still figuring out best practices.

---

Just found https://github.com/FormidableLabs/urql/pull/514, so you could fix it in a better way in urql. That's nice.

[sebmarkbage]

@FredyC The issue with the subscription based approach is what you do when the subscription fires after we’ve rendered that component but before we’ve committed it onto the screen and we get an update. React can do two things with that information. If React doesn’t immediately stop then the rest of the tree will get the new state and it will be inconsistent and cause tearing problems. If React does restart, it’s worse than the deopt approach because you eagerly throw away work even in cases where you wouldn’t need to. If you restart but keep it non-synchronous rendering you risk keep throwing away the work as you get new updates and you starve which is really bad.

So the subscription solution is actually a much worse deopt or it has the tearing problems.

Or to be precise it would be same performance as without Concurrent mode. That sounds more like a foot gun and not a solution.

Yes, this is indeed a foot gun but there’s not much we can do about that. A mutable state that mutates and then later dispatches a change notification that the state has already changed inherently has this property.

It’s a known property in systems that use threads too. Immutable data lets you about snapshots and allows work to be queued and parallelized. Mutable data requires locks and synchronization.

It comes with some downsides too and it’s a tradeoff. We can’t magically fix that.

The deopt approach is a decent compromise. At least it’s not really worse than synchronous React. It does make the story more complicated because you can’t really just say to use the same state for everything since that risks deopting too much. So that makes it a foot gun. It’s not React creating this tradeoff though. It’s just the realities of the tradeoffs of using mutable state.

[kitten]

@dai-shi Yes, but our solution is only possible because we can subscribe on mount and unsubscribe immediately which is our implementation for getCurrentValue, so we’re side-stepping the problem in our new approach.

We essentially stop listening between the first render (mount) and the commit phase. So this only works in our case because we can stop all side-effects in-between both phases.

@sebmarkbage Are the docs in use-subscription still correct about potential deopts there? As far as I understand that hook it doesn't cause any side-effects during render that trigger an update, but only in useEffect and in a listener callback. The comment in the issue seems to highlight the case where getCurrentValue inside useEffect may trigger a state update immediately. Is that what can also cause a deopt?

• https://github.com/facebook/react/issues/13186
• https://github.com/facebook/react/tree/master/packages/use-subscription#limitations-in-concurrent-mode

[markerikson]

@sebmarkbage : can we get a definition of what "deopt" _specifically_ means in this context? The React team has thrown that phrase around a lot, and it would help to know exactly what you're referring to.

Is it simply "a synchronous re-render during the commit phase", or something more specialized than that?

[kitten]

As @dai-shi pointed out in the other issue, this gist by @bvaughn clears things up in terms of what “deopt” means, when it happens, and what may cause tearing https://gist.github.com/bvaughn/054b82781bec875345bd85a5b1344698

If I’m not mistaken, “deopt” means that all updates triggered inside useSubscription‘s subscription or listener in the same tick will be batched and all changes will be committed and flushed immediately altogether. So for certain use cases, this is probably desirable actually, but may be expensive, because I don’t think this will update will be sliced, since the point of this big batch is to avoid tearing.

Tearing (which I’m more unsure of) may occur with useSubscription (and I suppose similar approaches) when either before this big commit phase or during it the subscription updates again. In such cases the newly mounted consumers of a subscription may temporarily be “more up-to-date” and additional update cycle will start.

This doesn’t seem to be an issue with useMutableSubscription, however this hook may trigger an interrupt by throwing and will restart the entire (then even larger) batch

[sebmarkbage]

@markerikson It can mean slightly different things in different implementation details. For example, it might not need to be in the "commit" phase. We can do it before the commit phase if we know it's going to happen earlier. It always means switching to synchronous rendering though.

It may also mean including more things in the batch than is necessary. E.g. normally React concurrent can work with multiple different pending transitions independently but deopt might group them into one batch.

For example if you have pending transitions loading it might also need to force those to be included into one big batch and force them to show early fallbacks. Such as pending startTransition. This can be an unfortunate experience if forcing fallbacks means hiding existing content while loading but you really just wanted to show a temporary busy spinner.

Another case we haven't built yet but that I'm thinking of is enter/exit animations. It might force the animation to terminate early.

I.e. new features that don't work with synchronous rendering can become disabled if we're forced into synchronous rendering. I think that's the worst part of the deopt. Not actually the rendering performance.

The cases _when_ deopts happen is also an interesting discussion point that varies by implementation details. E.g. the useMutableSource approach is meant to trigger the deopt less often.

[CaptainN]

Being able to deopt would be great for Meteor's legacy container and what should become the legacy hook, which can do a bunch of unknowable things in its general tracker. It will always create a computation, but might also fetch data, and or set up subscriptions. If there is a way to opt out of concurrency (deopting), that'd actually be ideal. We could create a separate set of specialized hooks which follow the rules in more specialized hooks to gain the purity benefits (which we probably should have anyway). I'm also looking at a new clean implementation of a general hook to support Suspense, and follow the rules of hooks.

[dai-shi]

While developing another library, I encountered this issue again.
(All cases for me are when using libraries that I don't have control. Yeah, just like all of us here.)

As I understand the current best workaround is to use timers, I made a small hook for it.
There's no deep thought, so it may contain a potential bug.
https://github.com/dai-shi/gqless-hook/blob/d2ee1778cbf3236932b219f332be877b2a5cfd3a/src/useCleanup.ts#L8-L26

export const useCleanup = () => {
  type Cleanup = {
    timer: NodeJS.Timeout;
    func: () => void;
  };
  const cleanups = useRef<Cleanup[]>([]);
  const addCleanup = useCallback((func: () => void) => {
    const timer = setTimeout(func, 5 * 1000);
    cleanups.current.push({ timer, func });
  }, []);
  useEffect(() => {
    const cleanupsCurrent = cleanups.current;
    cleanupsCurrent.forEach(({ timer }) => clearTimeout(timer));
    return () => {
      cleanupsCurrent.forEach(({ func }) => func());
    };
  }, []);
  return addCleanup;
};

Does it look OK?

[trusktr]

Without cleanup primitives for all cases (if part of a component runs, a final cleanup is a _must_) then this becomes needlessly complex for libraries built on top of React.

Any libs relying on timeouts, dual phase setup, or whatever, are now relying on React's internal implementation, which makes React less able to migrate to better implementation later without breaking libs.

---

All APIs (whether React, or not) should have creation APIs with reciprocal disposal APIs for _all_ cases. If everyone follows this principal for an API, then all libraries built on top of such API will always have a reliable way to clean up.

In React's case,

• render has the potential to allocate memory (especially when using third-party state tracking libs of any sort, no need to understand how they actually work).
• thus render should always have a guaranteed reciprocal cleanup method (f.e. useCancelledEffect or something like described above, or see idea below) where for every batch of render calls (or even just a single render call) there is always guaranteed to be a cleanup call (f.e. if a component renders at least once, then a cleanup call must always happen after no more render calls will ever happen).

Following these basic principals are the basis of good software design, whether that's in React or anything else.

---

Another idea is maybe update useEffect instead supply a cleanup function as a second arg instead of having an effect return it:

useEffect(
  () => console.log('instantiate some stuff'),
  () => console.log('destroy some stuff'),
)

then people interested in writing libs that depend on render use it like this:

useEffect(
  () => /*empty, this never runs if render is not committed*/,
  () => console.log('destroy some stuff from that first render'),
)

where useCancelEffect is effect the same thing but without requiring a first arg.

Main point: there should be _something_ that can signal to cleanup _if any part_ (at least any reactive part) of the component has ran.

[markerikson]

@trusktr : React _does_ have a place to do cleanups: in the effect hooks, which will always run _after a committed render_.

The entire point of CM is that allocating values that have side effects, while rendering, is not safe because that render might _not_ get committed.

I understand that you're asking for something _else_ beyond that, but people seem to be dancing around the issue that allocating things with side effects while rendering has never been correct. Well, now it matters.

[trusktr]

Seems like the React Hooks effort is battling against the JavaScript language.

State that will be used in a React component can technically be accessed from anywhere, for example an object with a property that is backed by an accessor that tracks state. And now React team has to explain to users why they shouldn't write code (or use any such library) that uses such language features for dynamic stuff.

Adding a simple hook (f.e. onCancelled) would be fairly simple for React to implement (I think) and it would prevent a lot of complicated code in code written on top of React (namely those state tracking libs). It's a win-win.

---

Now react basically has poor man's classes (React.FC, and worse than classes because now every "method" is re-created on every render), which is effectively the same as instantiating a class that extends React.Component, calling its render method, and never calling componentWillUnmount, which doesn't seem like a good idea.

---

In the code I write (in general, not talking about React) I always try to write explicit cleanup code that is always guaranteed to run after any object (or poor man's class scope) has been instantiated and when that object (or scope) is no longer needed.

This is a good practice that naturally leads to code that is easier to manage in the long run and in new and unexpected scenarios that may arise, and at worse a practice that can minimize the size of memory leaks (leaks are too easy to create, especially if never thinking about explicit cleanup, which is a far too common thing for novices to not think about).

[trusktr]

Besides all of that, isn't an uncommitted render one render too many? Maybe React, basically using poor man's classes, may as well have a useRender and the return type of React.FC should always be void.

[markerikson]

@trusktr No idea what you're trying to say with that last bit.

Here's the canonical reason why a render might get thrown away:

• React starts a low-priority render pass (say, due to an AJAX response coming in)
• React pauses partway through the render pass due to frame timing limits. It's called render() for half of the components in the tree so far.
• During the pause, the user presses a key in an input
• This starts a high-priority render pass. React executes that render pass immediately and commits it
• React now returns to the low-priority render pass that was interrupted. The input potentially changed a lot about what _should_ be rendered right now - maybe some components got unmounted or something.
• Therefore, some or all of the WIP component render output gets thrown away as React recalculates what the new UI should look like by applying the "received AJAX data" state change on top of the "input update" committed component tree.

So, where in that process is React supposed to do additional cleanup work, when it has no idea what side effects happened in _your_ code, and those renders were never committed?

The question here shouldn't be "how can I clean up work that got thrown away?" It should be "how do I do my work once I know that the component has actually mounted?"

If there are truly use cases that _aren't_ solvable with the current primitives that React provides, then the community should collect a list of specific use cases that are completely incapable of being solved so that we can figure out what primitives _are_ needed.

But so far most of what I seem to be seeing in all these different discussions is "I've been breaking the rules and now that's going to cause me problems, how can I keep breaking the rules without it causing me problems?".

[gaearon]

We’re focused on the “interruption” case but that’s not the only one where this matters. Another example you can think of is a hypothetical gesture API that generates keyframe animations between multiple states (e.g. an image gallery that you can scrub through with your finger). In this case we’d want to do several render passes to precalculate the keyframes, but none of them get committed until you lift the finger. So this notion of “cleanup” becomes really confusing because the thing you are “cleaning up” was never committed to the screen in the first place. It doesn’t have any particular lifetime.

The point we’re trying to make is that rendering is considered a process that should not allocate resources beyond what a traditional GC would collect. Because even if we built a user-space collector for it, it would have poorer performance characteristics and undermine the ability to do these renders fast. Since now we can’t just throw away trees. This undermines all the features that we’re building on top of it.

That said, you could just rely on GC eventually. The WeakRef proposal lets you add some cleanup logic. Its timing would not be deterministic, but neither would it be for a similar React-managed mechanism. So this seems like the longer term solution for the rare cases where you can’t use an effect and for some reason have to allocate a resource during render.

[trusktr]

I think of the canonical reason why a render might get thrown away as the following:

• because there's no way to prevent a React.FC from returning something, so basically they always "render" because they always construct a vdom tree. Otherwise, with an idea like useRender in my last comment, React could selectively skip renders for any of the reasons you listed (saving CPU and memory cost, and avoiding the above problem in a different way).

So, where in that process is React supposed to do additional cleanup work, when it has no idea what side effects happened in your code, and those renders were never committed?

Why should any library like React ever need to know about side effects in _my_ code? It should never need to know. It should only provide a guaranteed cleanup method (after all, it's (poor man's) classes at this point).

"how do I do my work once I know that the component has actually mounted?"

When using only the React APIs, then that question is already answered with useEffect. But as mentioned above, people are making other libraries that are activated by a React.FC being called and there being cases when the FC is called and no cleanup opportunity is presented.

This problem is the same as a class constructor being ran and instantiating third-party state tracking libs, but no reciprocal cleanup method ever being called and thus those libs tracking state forever. That's exactly what the body of a React.FC is: the constructor of a poor man's class, and React not every calling a cleanup "method" at any point in the future if that component has not been "committed".

---

Imagine writing all C++ classes without destructors. That's the same issue.

[gaearon]

But as mentioned above, people are making other libraries that are activated by a React.FC being called and there being cases when the FC is called and no cleanup opportunity is presented.

We’re going in circles but let me reply to this. People can write any kinds of libraries — we can’t stop them. But the React component contract has always emphasized that the render method is meant to be a pure function of props and state. This makes it throwaway-friendly. It is not intended as a constructor, in fact it’s the other way around — it’s React classes that act as poor approximations of functions. Which is why React is converging on a stateful function API now.

If a library chooses to ignore the rules and mutate a list of subscriptions during render, it is its business. But it shouldn’t be surprising that when we add new higher-level features to React that rely on the component contract having a pure render method, those features may not work. I think it’s fair to say that render being pure is well-documented and one of the very first things people learn about React.

[trusktr]

That's all fair enough. It's React team's library, and React team has the right to say how React is supposed to be used and does not need to support everyone else's alternative paradigms.

---

That being said,

It is not intended as a constructor

That may be the intent, but by providing ways to define method-like things (f.e. useEffect) the FC body has become _similar_ to a constructor because the "instance" (the lexical scope) has the ability to have long lived things associated with it after creation (normally those things are created in something like useEffect, not in the function body itself) including DOM objects.

Actually, the "instance" is the cumulative set of scopes from every time that the FC has been called in its life time (a very important but subtle difference, sometimes a confusing one).

The similarity of FCs to classes may be an unintentional invitation for code that creates state at the beginning. The render (f.e. the FC return value) is the only thing that an external dependency-tracking system can rely on for tracking dependencies used for rendering.

---

Something like onCancel (or whatever name it would have) seems like a very easy way to solve this problem.

[LXSMNSYC]

To solve this, what I did is to treat my own objects to have a dispose/cleanup logic, and created a hook similar to what @dai-shi created:

function useDispose(dispose) {
  // schedule a cleanup, 16 could be a good number.
  const timeout = setTimeout(dispose, 64);
  // cancel the scheduled cleanup if the side-effects went through.
  useEffect(() => {
    clearTimeout(timeout);
  }, [timeout]);
}

This worked, in my case, specially just to prevent an external source (e.g. an object that subscribes to multiple events) from subscribing twice.

[Andarist]

Actually, the "instance" is the cumulative set of scopes from every time that the FC has been called in its lifetime (a very important but subtle difference, sometimes a confusing one).

This mental model is not correct - some render calls can be freely dropped by React so it's a false assumption to think about an instance being a cumulative set of scopes from every time that FC has been called in its lifetime. If anything it's a cumulative set of committed scopes. This distinction is very important in understanding how to obey React rules.

It has been mentioned here in the thread already - but those rules are old. They just start to be way more important now but breaking those rules probably has caused mem leaks and similar problems in the past already.

I can't say about the community as a whole but it has always been quite clear for me that constructor in a React's class was not an appropriate place for side-effects and while it was maybe annoying at times I just have always used such logic to componentDidMount and usually had no problem with that. It was a simple refactor.

[bvaughn]

Adding a simple hook (f.e. onCancelled) would be fairly simple for React to implement (I think) and it would prevent a lot of complicated code in code written on top of React (namely those state tracking libs). It's a win-win.

You are right that it would be easy to add such a hook. The reason we haven't done it is that it would have a significant negative impact on performance.

I think of the canonical reason why a render might get thrown away as the following:

• because there's no way to prevent a React.FC from returning something, so basically they always "render" because they always construct a vdom tree. Otherwise, with an idea like useRender in my last comment, React could selectively skip renders for any of the reasons you listed (saving CPU and memory cost, and avoiding the above problem in a different way).

Unless I'm reading this comment wrong, I think it's a misconception. A component can already return null to say that it does not want to render anything. That is not the reason a render may be thrown away. There are a few reasons why React might not commit the value rendered by a component, including:

• Something else scheduled a higher priority update (an interruption)
• Another component threw an error
• Another component suspends to wait on loading data
• The pre-rendering animation use case Dan mentioned above

[kitten]

@bvaughn @gaearon Are you considering a system for push-based subscriptions that coöperate with the component lifecycle by any chance? I still find it to be a little odd that most systems are rather pull-based once they instantiate their state, i.e. outside of effects we prefer to be able to get either "snapshots" (useMutableSource) or the state directly (useSubscription). The same goes for suspense, it's all rather dependent on having pull-based state/effect sources.

I think a lot of problems could be solved if there was a way to make push-based systems work better together with React's hooks & lifecycles. This would indeed be rather novel, but I think it'd fix this. This is especially important for derived event sources, where the state isn't pull-accessible. Currently in urql we work around this by creating a state snapshot by subscribing & unsubscribing to our source to get a current one-time snapshot of our state.

However, if state was tied to some kind of observable source from the start of a component lifecycle, similar to how useMutableSource prevents tearing and forces the source into a more coöperative timing, it'd have a good method of ensuring that a synchronous event would lead to a valid initial state and that subsequent events would have a sufficient amount of backpressure applied to them — in other words, a push-event would still act just like a normal state setter today.

[CaptainN]

If there are truly use cases that aren't solvable with the current primitives that React provides, then the community should collect a list of specific use cases that are completely incapable of being solved so that we can figure out what primitives are needed.

Here's the issue in Meteor. Meteor uses a computation for setting up observers on "reactive" sources, based on accesses to those sources on the first run of the computation function. It does that in the render pass now, with some caveats. We could simply turn off the observer for the render pass, then run it again in useEffect - in this case it would always need to run again, because state might change between render and useEffect. We currently use timers to try and avoid that, but we already had to disabled that for one case to support StrictMode, and it's proving unmaintainable.

The question is, if we have to run our setup code a second time on commit, is it really earning performance, or just moving the performance hit off to other parts of the app? Then again, in most cases in Meteor, it's probably not a big deal to run the computation again - most computations are relatively cheap, unless there is some huge complex mini mongo query or something

I went down the rabbit hole of trying to pull that out in to a data source, as described in the "suspense for data" section in the React docs, but that has a number of problems. Primarily, it complicates what is otherwise a clear and easy API for us (with useTracker - except for deps, developers regularly mess up deps), but it also felt like I was just picking up that kicked down the road performance work, and things were getting COMPLICATED. Ultimately, I couldn't figure out a real great way to make an externalized API like that work because of various scoping issues, and the complexity of basically rebuilding redux with a sort of dependency injected inversion of control system (which is where I was headed) felt like a step backwards.

[markerikson]

@CaptainN : fwiw, that "might need to run again because state could have changed before the effect" part sounds _exactly_ like how the new useMutableSource hook is intended to work. Have you looked at that at all?

[arcanis]

I've also hit a few times the case of a component needing to instantiate objects tied to the WASM heap. In those cases there is no garbage collection to rely on for freeing uncommitted resources (not yet, at the very least), so the only choices seem to be to enter a world of double-renders with useLayoutEffect, or to break the CM assumptions. I'm currently doing the first, but it seems prone to degraded performances should those components be nested a bit too much.

[edit] I made a search on "WASM" before sharing my experience, but I think the following comment is pretty much what the React team would answer to that: https://github.com/facebook/react/issues/15317#issuecomment-716210926

That said, you could just rely on GC eventually. The WeakRef proposal lets you add some cleanup logic. Its timing would not be deterministic, but neither would it be for a similar React-managed mechanism. So this seems like the longer term solution for the rare cases where you can’t use an effect and for some reason have to allocate a resource during render.

Unfortunately it'll be years before WeakRef reaches general availability. I guess it can work if CM takes years too 😛

[CaptainN]

@markerikson I did look at that a few months back, but wasn't sure exactly what that does or how (or if it was ready for use) - is that finalized or still in RFC? Last time I looked at it, I thought it would drop any component using it in to a "deopt" mode, opting out of the benefits of concurrent mode for that component, and wanted to see if I could keep the benefits.

It's on my list to pursue a useMutableSource based solution for a future version the generic useTracker (or simply eat the second setup pass) while moving to a new more tailored set of hooks to integrate React with Meteor, which can be implemented in the pure way, without a ton of complexity. Those hooks already look better.

[markerikson]

@CaptainN : useMutableSource has been merged in, but I _think_ it's still only available in "experimental" builds. The "deopt" behavior as I understand it only kicks in in certain situations.

Not saying it's going to magically fix your problems, but I think you should go review the RFC and PR discussions around it and give it a shot to see if it does potentially address your use case.

[brucou]

But the React component contract has always emphasized that the render method is meant to be a pure function of props and state.

@gaearon Functions components that use hooks are not pure in the most general cases (i.e., unless you find a hook that is actually pure). I am not sure what is the render method you are referring to. I assume you are referring to the render method use in React's class API? That render method should indeed be a pure function of the parameters it is passed, as it is documented. But then there seems to be some confusion in your next assertion:

But it shouldn’t be surprising that when we add new higher-level features to React that rely on the component contract having a pure render method, those features may not work. I think it’s fair to say that render being pure is well-documented and one of the very first things people learn about React.

The component contract you are referring to is the class component contract. If you get rid of class components, you get rid of that contract. Then function components (if they use hooks) renounced to purity so you can't really refer there to a pure render here. Given that functional components and concurrent mode are a new thing, why not clarify the new contracts for the new thing? That takes possibly wrong assumptions out of the way together with the bugs they create. This especially applies to concurrent mode as it seriously changes what was assumed to be a contract in former React versions. Some people continue to think that it is React as usual apart from cosmetic changes but it is really a fundamental change. I am not sure this has been captured yet in the documentation.

[bvaughn]

I am not sure what is the render method you are referring to.

class ClassComponent extends React.Component() {
  // ...

  render() {
    // This is a render method and should be pure.
  }
}

function FunctionComponent(props) {
  // This is also a render method and should be pure.

  // Built-in hooks (e.g. React.useState) are an exception
  // because they are explicitly managed by React
  // and so are more like locally created variables.
}

[benjamingr]

Can't MobX just use a FinalizationRegistry @mweststrate ?

[brucou]

@bvaughn The second case (FunctionComponent) is a function. Methods usually refer to functions within classes (or by extension objects). Then, again, it is not accurate to say that function components should be pure when them being impure is far from being an edge case (due to hooks usage). There is no such thing like an impure function that is like a pure function except this or that. People get confused precisely because we use the same words to mean different things. In the context of computer science, functions that may be impure are usually referred to as procedures. So you may call a function component a render procedure if you will, but not a pure function in a large majority of cases (when they use hooks). Which is one of my points and the reason why people get confused by functional programming notions decaffeinated with exception and metaphors.

[bvaughn]

I'm not looking to debate the semantics of "methods" or "functions", just addressing your comment that you were "not sure what is the render method". The answer is: Dan was referring to both class components and function components.

[gaearon]

I hear that the documentation does not do a good enough job defining “purity”. This is something we’re keeping in mind for the upcoming docs rewrite.

On the other hand, if you take the technical argument that “function components are impure because of Hooks”, I don’t think you can claim that “the class render method is pure” either. After all, it reads this.props and this.state which are by definition properties on a mutable instance — thereby making any render method technically impure. This technicality hasn’t prevented people from understanding that the rendering process in React (whether with a class method or a function component) is meant to be generally pure. But we could definitely be stricter in our definitions and this is good feedback.

[brucou]

@gaeron you are indeed right. The render class method is often impure too. That escaped me. I thought it received props as an argument, but it is true that it gets it from this. Actually often the reason why you would use a class is precisely to do this kind of impure things that pure functions does not let you do (this.state). Your point is well taken.

But still, whatever it is that you mean by the rendering process is meant to be generally pure can probably be expressed in a more precise (albeit more wordy) way. Did you mean something like the vDom should be (generally) computed only from props, context and children --- independently of how you actually get that information, and that effectful components should be the exception and non-effectful components the rule? There is a pattern like that, which combines renderless components and pure components. Or did you simply mean that React functions components can do all kind of things as long as the computation of those things only depends on props, context, children etc and a variety of React APIs? That is, the output (understood as effect executed + computed return value) of function components depend only on inputs and the React runtime state? So basically you don't do effects that do not go through a React API.

With concurrent mode, I believe that accuracy is going to be even more important. The hard part is going to have developers build the mental model that shortens debugging sessions and reduce the need for technical support. Of course, examples of usage will help a ton and you have good stuff already on the documentation site. But having consistency in words and meaning (we call that ubiquitous language in TDD) goes a long way too. It was for instance important that React introduced different words for rendering and for committing in order to explain the different operations that React does at different moments and it would be unfortunate to mix one with the other randomly.

[mweststrate]

Random update: in mobx-react-lite we are now experimenting with using a FinalizationRegistery as suggested (and executed) by @bnaya / @benjamingr. So far it looks quite promising and something that could be abstracted away in a generic hook (possibly with a timer based fallback).

The two liner summary of the approach is that in the component some a local object is allocated and stored using useState: const [objectRetainedByReact] = React.useState({}). With a finalizationRegistery we can then register a callback to get a notification when the objectRetainedByReact is garbage collected by the JS engine, which should be a safe heuristic that a an instance wasn't committed.

[dai-shi]

Interesting. So, it doesn't even check if it's committed with useEffect. Should we expect JS engine not to garbage collect objects very soon if there's enough free memory?

[mweststrate]

It does useEffect to mark comitted, left all the implementation details out
😉

On Tue, 3 Nov 2020, 22:31 Daishi Kato, [e-mail] wrote:

Interesting. So, it doesn't even check if it's committed with useEffect.
Should we expect JS engine not to garbage collect objects very soon if
there's enough free memory?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/15317#issuecomment-721408206,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHHYJZDCAVPTSM4DBDSOCAEPANCNFSM4HDRIAHQ
.

[benjamingr]

Just a small correction, I suggested using FinalizationRegistry (for MobX) - the execution is 100% @Bnaya :]

(the idea came after discussing the idea ±2 years ago for MobX in some JSConf with Daniel Erhenberg in the context of "how should Node behave" and I figured "what other libraries am I involved with and may use this")

Michel had an interesting idea (to make this into a generic hook - useDisposer or something) which I believe would be very cool.

@gaearon I think it would be very useful if you can't commit to cleanup or when cleanup runs if React provided this sort of hook as part of React since the use case is pretty common. Is that something you'd be willing to explore or is that antithetical to the direction you want to see React take?

In any case I think it's better to discuss this once @Bnaya is further along with the useDisposer work.

[dai-shi]

Cool. I was going to draft something like useDisposer with FinalizationRegistry, but will sit and wait. My two cents is this is still like an escape hatch and is not something like a first recommended pattern.

[Andarist]

Is using FinalizationRegistry really a good idea? I mean - I understand some part of the frustration coming from people wanting this kind of a feature, I also understand why this is not something that the React team wants to support (unless compelling arguments are found). However, resorting to things like this is a huge bandaid, a hack. I think that no one even would question that. So if this is a hack (and not a small one) - is it worth pursuing? I feel like either idiomatic React approaches should be used - which for some use cases are at least quirky to deal with, I don't have super compelling use cases for myself regarding this as, usually, I can rather easily work around this or convince the React team that this is really needed for the community. It's obvious that the React team understands challenges and the use cases mentioned here but those deviate from their model and they are hesitant to implement this as it can actually be a footgun if not used properly, but maybe it's time to gather the people most interested in this in sort-of a focus group that would figure out the path forward? Maybe this happens somewhere in the background - I don't know, but at this point in time this really seems like a stagnant issue. A pretty much the same thing is repeated all over the place from both sides and the discussion seems kinda fruitless.

If no consensus is found then I expect such hacks to be actually implemented and used, not because I feel like that they are needed that much but just because of human nature. We all have strong opinions about things and it's inevitable that something like this will exist in the community eventually, question is - can we prevent it? Or at least eliminate it from being needed for more use cases?

Given the recent work on the docs - some of the use cases mentioned here should probably be documented there to present what's the React team's stance on them, what are proposed solutions.

[markerikson]

@Andarist fwiw, my take is that this might be a "hack" in the sense of "well, this really isn't how you _ought_ to be writing your code in the first place based on the rules", but it may very well be a valid technical solution for MobX's use case and potentially other folks as well.

Given that, I wouldn't expect the React team to specifically _recommend_ this approach, but perhaps it can be published as an independent package by whoever's writing it, and the React team could maybe at least document it as a known workaround.

[sebmarkbage]

Especially as an incremental upgrade approach can be a valid strategy. As new research is going into new patterns.

However, there's one interesting case to think about. The main motivation for FinalizationRegistry to be added to the language _now_ of all times, is because it allows building interop with WASM linear memory (aka array buffers). In a more general sense, it's how you build interop with a tracing GC language and a ref count or manual collection GC language.

For any library implementing its backing store in a WASM world, this would be the primary way to interop with it.

There are many patterns that might be a bad idea around this (like opening web socket connections without deduplication until the GC runs). The core principle of using FinalizationRegistry to interop with an external memory management library isn't though. That's the point of FinalizationRegistry in the first place.

[Bnaya]

I've created useDisposeUncommitted hook, similar to MobX's impl
https://www.npmjs.com/package/use-dispose-uncommitted
I've also opened a self-PR to collect feedback on the initial version and api
https://github.com/Bnaya/use-dispose-uncommitted/pull/3/files