Prefer using react api for useProperty hook

[PalmZE]

Hey, @raveclassic!

Intro

I'm developing a chat. It means that I need to subscribe to properties in leaves (messages, etc.) so only necessary parts re-render.
I was testing performance and noticed that old version of useProperty lost updates (new works fine). This led me to some research. Results are below.

1 React-way to subscribe to mutable sources

Look like there are three versions of "how to subscribe to external mutable source" in React:

1. createSubscription + useSubscription
2. useMutableSource (was renamed to the below)
3. useSyncExternalStore (package)

It turns out that managing frp-ts like source is not that easy in React because you have to take into account SSR, Suspense and async rendering (coming in React 18).

My suggestion is to use useSyncExternalStore in frp-ts/react package so the React team handles all the details (I'll attach reading list at the end for a better context).

2 State modification during render

I know that this is an anti-pattern and it should not be done (react repo issue). But, now I wonder whether this still can happen in large codebases (sockets, event handlers, whatever).

React team introduces StrictEffects and this should catch the above. Until that lands, I wonder whether deferring state.set calls as done in most eliminates the problem completely.

I still need to wrap my head around this. Maybe this case will never happen in a real application. Maybe the only way to trigger this is to directly modify the state in a render function.

Two demos showcasing this:

• frp-ts
• MobX

3 Further read

Show

Packages:

• use-subscription
• create-subscription
• use-sync-external-store

API Discussions:

• use-mutable-source
• use-sync-external-store
• how to support strict effects

react-redux migration to useSES:

• v8 release-notes
• prep for React18
• useMutableSource

mobx issues:

• react requires immutability
• supporting react concurrent

New version of the useProperty hook

I tested my component with the hook below and everything seems to work fine.

const bridge = <A,>(
  p: Property<A>,
): {
  subscribe: (onStoreChange: () => void) => () => void;
  getSnapshot: () => A;
} => ({
  getSnapshot: () => p.get(),
  subscribe: (onChange) => {
    const sub = p.subscribe({
      next: () => onChange(),
    });
    return () => sub.unsubscribe();
  },
});

const useProperty = <A,>(p: Property<A>): A => {
  const { getSnapshot, subscribe } = useMemo(() => bridge(p), [p]);
  return useSyncExternalStore(subscribe, getSnapshot);
};

Summary

1. Looks like useSyncExternalStore is the way to go for frp-ts React integration.
2. There is a potential issue with mid-render updates, maybe there is a smart way to prevent\fix that (not necessarily in frp-ts).
3. Maybe there is some works needed to prepare for React 18.

[raveclassic]

Hey @PalmZE, thanks for such a detailed issue!

So, correct me if I'm wrong, the issue is that during rendering of list items that reference list values in Properties by index, we can get out of sync with the data because this list item component is not unmount in time, right? This happens due to asynchronous nature of the renderer, right? Then I would expect such components reading the data by index to check whether the item is not undefined. That would be the correct behavior to work with changing lists and "stale" indexes. By "stale" I mean that they might get out of sync due to async rendering in React. That's a common problem actually and it's not related to setting the state in the render cycle. The wrong index, captured during rendering of the list, might reference non-existing item anyway, as the rendering most of the times is out of sync with the data (especially taking React's Concurrent Mode into account). So, in short, I would say it's the responsibility of the UI composition to write to/read from Atom/Property correctly.

Originally, @frp-ts/core was design to be fully synchronous at its heart, so although most and other libs implement asynchronous value delivery (microtasks for example), I'm not sure whether it's a good option to adopt similar concept here. At the end of the day, it's not the responsibility of Atom to think about how/when the value should be delivered to its consumers, but of the caller of Atom#set. In most cases, synchronous emission is enough, in some cases (like with React) it may break things but we should think about such cases outside of the core. For example, it's fine to tackle this in @frp-ts/react.

Ok, now speaking about @frp-ts/react, honestly I don't know wtf is use-subscription/``use-mutable-source` or any other fancy APIs they are going to bake into the core :D. But, seems until it's not in the core, we are free to ignore it. At the same time, I think it's fine to think about improving existing bindings to React if there are any issues with them.

Thanks for opening the issue and let's continue the discussion here.

[PalmZE]

So, in short, I would say it's the responsibility of the UI composition to write to/read from Atom/Property correctly.

I thought a bit more about the issue, and I agree.

At the same time, I think it's fine to think about improving existing bindings to React if there are any issues with them.

Existing bindings are working fine. This is an approach similar to other libraries (mobx, effector, etc).

I created this issue to discuss the possibility of using useSyncExternalStore since it simplifies the usage of atom-like sources. But, I agree that until this api is in the core (and React 18 is released) current version is safer.

I'll test useSyncExternalStore in my own project. Can let you know, if it works ok in the long run.

---

Conclusion: I have nothing to add, feel free to close this issue.

[raveclassic]

Thanks for the feedback @PalmZE. Closing.