Closed PalmZE closed 2 years ago
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.
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.
Thanks for the feedback @PalmZE. Closing.
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:
createSubscription
+useSubscription
useMutableSource
(was renamed to the below)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
infrp-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 deferringstate.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:
3 Further read
Show
#### Packages: * [use-subscription](https://github.com/facebook/react/tree/main/packages/use-subscription) * [create-subscription](https://github.com/facebook/react/tree/main/packages/create-subscription) * [use-sync-external-store](https://github.com/facebook/react/tree/main/packages/use-sync-external-store) #### API Discussions: - [use-mutable-source](https://github.com/bvaughn/rfcs/blob/useMutableSource/text/0000-use-mutable-source.md) - [use-sync-external-store](https://github.com/reactwg/react-18/discussions/86) - [how to support strict effects](https://github.com/reactwg/react-18/discussions/18) #### react-redux migration to `useSES`: - [v8 release-notes](https://github.com/reduxjs/react-redux/releases/tag/v8.0.0-alpha.0) - [prep for React18](https://github.com/reduxjs/react-redux/pull/1808) - [useMutableSource](https://github.com/reduxjs/react-redux/issues/1762) #### mobx issues: - [react requires immutability](https://github.com/facebook/react/issues/20064#issuecomment-713181739) - [supporting react concurrent](https://github.com/mobxjs/mobx/issues/2526)New version of the
useProperty
hookI tested my component with the hook below and everything seems to work fine.
Summary
useSyncExternalStore
is the way to go forfrp-ts
React integration.frp-ts
).