Mobx: Support concurrent mode React

[mweststrate]

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

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

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

[mweststrate]

That is: assuming no one is using concurrent mode in production atm

[FredyC]

For anyone interested mobx-react-lite@next has experimental support for Concurrent mode. Feel free to test it out.

Personally, I don't think it's worth the hassle supporting Concurrent mode in class components.

[bsmith-cycorp]

They just published a post getting further into details about Concurrent Mode: https://reactjs.org/docs/concurrent-mode-intro.html

[FredyC]

I believe we are ready for Concurrent mode with mobx-react-lite@next. Of course, at some point, we shall try against "experimental" version if tests really do pass just to be sure. PR for that is surely welcome :)

[bsmith-cycorp]

I hadn't heard about mobx-react-lite.

Is it intended to supercede mobx-react? That would be hugely disappointing for me, as I take serious philosophical issue with hooks, especially given MobX's emphasis on (and synergy with) classes.

[FredyC]

Um no, mobx-react@6 is built on top of mobx-react-lite :)

[mweststrate]

Ok, FWIW, using suspense with observables is pretty natural if you throw when(() => expr) :)

https://codesandbox.io/s/dry-microservice-qnle3

[FredyC]

@mweststrate That's a certainly nice feat, although I never liked much to have fetching logic inside the stores. But I guess with Suspense it will make more sense to have it separated from components for a proper preloading.

I also love that it will re-render on further updates to those observables without any extra magic. That makes it really powerful.

That fetchingPosts extra state is awkward though, considering Suspense should kinda remove the worry about any sort of loading state, it shouldn't be needed. I do understand why it's there in this case, but it's not ideal for sure.

[mweststrate]

Suspense doesn't make any decisions on who triggers rendering. So I didn't
do either in the example.

THe first is triggered 'externally', the second is triggered by rendering,
just to demonstrate both approaches.

The loading state again is a design decision. Should every render trigger a
new fetch? Only if not pending? Or only at mostly once? The loading state
was introduced to demonstrate you can express either solution you want.

On Fri, Nov 8, 2019 at 10:57 PM Daniel K. [e-mail] wrote:

@mweststrate https://github.com/mweststrate That's a certainly nice
feat, although I never liked much to have fetching logic inside the stores.
But I guess with Suspense it will make more sense to have it separated from
components for a proper preloading.

I also love that it will re-render on further updates to those observables
without any extra magic. That makes it really powerful.

That fetchingPosts extra state is awkward though, considering Suspense
should kinda remove the worry about any sort of loading state, it shouldn't
be needed. I do understand why it's there in this case, but it's not ideal
for sure.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-react/issues/645?email_source=notifications&email_token=AAN4NBD6TCLMJWCTOFZSIXDQSXVFBA5CNFSM4GWQEUKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDTTHCQ#issuecomment-552022922,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBFSKUHEMVSFMNZL3WLQSXVFBANCNFSM4GWQEUKA
.

[FredyC]

@mweststrate It's certainly a big mind-shift for everyone.

I have modified your example to utilize the useLocalStore which drags along a scary story it won't work with Concurrent mode. In this contrived example, it seems to be working just fine. Can you perhaps show what you had in mind back then?

https://codesandbox.io/s/mobx-suspense-e3rz4

[mweststrate]

Yes, I'd expect useLocalStore not to work with suspense, at least the model would be very hard to reason about even if it worked. As stated before I recommend using Reacts primitives for local components to benefit from Suspense, and mobx for the external 'domain state' which should, unlike UI state, never be in flux (forked). I'll update the mobx.js.org observer documentation to reflect that make that more clear :) But that is one of the reason I really didn't emphasize the mobx-react hooks on https://mobx.js.org/refguide/observer-component.html

[FredyC]

My wires must be crossed, but it's same old cryptic "it won't work" without explaining what is supposed to be broken :)

as this can theoretically lock you out of some features of React's Suspense mechanism.

And practically it means what? :)

For me the main reason to utilize useLocalStore is essentially so I can pass that store down the tree and have it coupled with actions/computeds. So yea, I don't use it strictly as the local state, but more like decentralized state management :) Is there some reason why should Concurrent/Suspense cause problems? I am failing to see it.

It would be great to have some real explanation in https://mobx-react.js.org/ as well since it's more future-oriented and I believe people are slowly using it more often as a source of information.

[mweststrate]

I never build anything to prove it, but if I understand suspense correctly, it will fork render trees when something is suspended, and something else isn't. That means that a component can exists twice at the same moment in time. That means that if you use useLocalStore, that store can be created twice, which can easily lead, at least, to a lot of confusion. Furthermore, when a tree is finished, suspense will (again, didn't see a concrete example of that) rebase the state updates (that is why the function version of setState is so important). However, rebasing observable updates is not really generically solvable. 

I think the interesting thing to test in your example is what would happen if that localStore is created somewhere in a component _inside_ a suspense boundary, will you get two stores in that case (at least temporarily)? Now it is created outside, so I'd expect that case to work flawless indeed.

So, I expect, for useLocalStore to work properly, you'd need something that 'shares' the state across all instances of that component instance, but afaik React doesn't really offer a primitive for that (although it could maybe created by using a dedicated context for that an a special hook that stores them in there, if instances can be uniquely identified from within the component)

Beyond that, a very simple example where things get weird immediately is that useDeferredValue wouldn't work with a store created by useLocalStore, because both the deferred value and the current value would point to the same object (since mutations don't create new references to the store) 

[mweststrate]

Relevant thread: https://twitter.com/acemarke/status/1230687035045896192

[FredyC]

Might be not relevant anymore too, so let's close it till there is someone who wants to investigate this.