fable-compiler / fable-react

Fable bindings and helpers for React and React Native
MIT License
275 stars 67 forks source link

memo vs. lazyView #138

Closed cmeeren closed 5 years ago

cmeeren commented 5 years ago

In https://github.com/fable-compiler/fable-react/issues/136#issuecomment-453788048, @vbfox said this about memo vs. lazyView:

Essentially yes memo is a react function to create components that act like lazyView does in elmish, only they are separate react components and that can be better in a few cases.

The diff algorithm fast-path when it see different components before and after a change considering their DOM tree different enough to drop the previous one and re-create instead of trying a mutation and lazyView hide that fact. They also appear nicely in the react debug view instead of it being a sea of lazyView ;)

But that too is essentially more of a detail, Using lazyView/memo/shouldComponentUpdate is the most important optimization to start with when the basis is an elmish application doing only direct view composition.

I get the last point, but I still find myself wanting to understand more about the details of the difference between lazyView vs. memo, and when one might want to use one over the other, because to the best of my understanding I still can't decide between them.

My current understanding

I feel I'm on thin ice here. Is the following correct?

lazyView

memo

Again, all statements above are to be taken as my current understanding, not fact. Please let me know of any misunderstandings.

Question 1

One thing I know I don't understand at the moment is this part of the comment by @vbfox (emphasis mine):

The diff algorithm fast-path when it see different components before and after a change considering their DOM tree different enough to drop the previous one and re-create instead of trying a mutation and lazyView hide that fact.

How does lazyView hide that fact? If lazyView wraps a child view whose top-level component changes from fooComponent to barComponent (based on the state), won't the React diff engine recognize the fact that the component has changed and thus drop and re-create instead of diffing recursively? If the emphasized part above is true, doesn't that necessarily have to apply to any component at any level below the top-level lazyView? (Which just seems insane to me, so I might have misunderstood the comment.)

Question 2

Going forward, which of the two functions is actually recommended to use? It seems to me that they have almost exactly the same use-case, and as far as I can see, they can both be tuned (using their ...with variants) to behave more or less identically, without too much boilerplate (by defining one or two simple "reflection-based" equality comparers that either does prop-wise reference equality (for lazyView) or ignores dispatch and other non-Elmish state (for memo)).


(Sidenote: Sorry for taking up your time with all the questions I have posted lately. I have considered asking them Stack Overflow, but I don't think it's a good fit since I don't fully know what it is I don't understand, so there's a lot of back and forth, follow-up questions, etc. For what it's worth, I think the questions could be relevant pointers for future documentation.)

vbfox commented 5 years ago

Question 1

I won't repeat my fableconf talk here but essentially let's take a sample :

If you move from a userView to a groupView both using lazyView react will see no diff there, so it will continue inside, finding in both a div (most of the time) so it will mutate that div, uselessly, then inside it will find a h3 or another title that it will also mutate, then it will find a li with the characteristic of the user or group... that it will diff recurse into, the elements will be different and for some it will diff other it will recreate

Obviously it might have stopped before, userView might have an avatar where the group view had a list and so less elements will be diffed, but it's mostly letting that to random chance (and so will provide very variable performance) while there is a good way to hint react about this : Introducing a semantic level in it's tree, a real GroupComponent and UserComponent.

Question 2

Ideally once memo is in a stable version i'd like to introduce (Either in react elmish or as a separate package) something that look like memoBuilder but take an additional parameter for dispatch and dispatch based functions. It would produce a component at the react level that take the props as-is with an additional __dispatch property that a custom diff function would ignore.

As the value would be of any type, a tuple or custom record could be used for passing functions built from dispatch.

It would also be usable for passing dispatch itself but the stabilized version in Elmish 3.0 can also just be a prop.

vbfox commented 5 years ago

Also @alfonsogarciacaro version of a diff that simply ignore functions (in #137) could be even better that my alternative.

Anyway once the dust settle on current previews i'm sure we'll end up with a Elmish specific builder with a signature identical or similar to memo. But how exactly isn't 100% certain.

cmeeren commented 5 years ago

Thanks for taking the time to help clear up things!

Question 1

Ah, I think I understand. Let me check – if I have understood correctly, the following should be true:

Is this correct?

Question 2

So your recommendation going forward is to drop lazyView and use memo, which should ideally have some helpers somewhere to make it work better with an Elmish architecture, is that right?

By the way, when you say

once memo is in a stable version

then you're talking about Fable.React's memo, right? (I assume the React memo API is stable?)

vbfox commented 5 years ago

Question 1

Essentially no :)

lazyView doesn't eat up the top component it is a component from React point of view an all it's children are normally rendered.

But it's always the SAME component type.

let userView state = lazyView (fun s -> ...) state let groupView state = lazyView (fun s -> ...) state

Going from one to the other isn't a drastic change in react because both are a LazyView component.

If you have a lazyView with inside a GroupComponent and replace that with another that has an UserComponent it will work, the diff will go one level deeper than if memo had been used but it's not a big deal.

The only question here is why use lazyView instead of these components inheriting from PureComponent ? If you have real components for part of your page that represent business concern (and not technical ones like button or div) then lazyView isn't useful.

It's also not the typical elmish app where no custom user-defined components are present.

Question 2

Yes React itself moved on for some time now :)

cmeeren commented 5 years ago

Question 1

Thanks for the clarification! A couple of follow-up questions:


If you have a lazyView with inside a GroupComponent and replace that with another that has an UserComponent it will work, the diff will go one level deeper than if memo had been used but it's not a big deal.

It seems to me that you're saying more or less the same as my original remark:

If lazyView wraps a child view whose top-level component changes from fooComponent to barComponent (based on the state), won't the React diff engine recognize the fact that the component has changed and thus drop and re-create instead of diffing recursively?

Was my original remark correct, then? In other words: Is it correct that lazyView doesn't "hide that fact" as you originally said, instead it just inserts one extra level of diffing?


The only question here is why use lazyView instead of these components inheriting from PureComponent ?

Yes, you're right. If one defines class components in the first place, then AFAIK PureComponent should serve the same purpose as lazyViewWith does for "sub-views" (with a suitable shallow diff function). My example was a bit artificial because it was only intended to test my understanding of how lazyView works.


If you have real components for part of your page that represent business concern (and not technical ones like button or div) then lazyView isn't useful.

Is this because lazyView serves the same purpose as PureComponent, which would be a better option than lazyView? (If you meant something else, I'm afraid I didn't understand this particular comment.)


It's also not the typical elmish app where no custom user-defined components are present.

Yes, you're right. A suitable Elmish-specific memo builder could solve this, right? Hide all the diffing and prop construction and "just work" like lazyView does? (Edit: I don't think it can, because AFAIK you need to call it only once outside your render function...) Though I can't immediately think of any way to implement a suitable "shallow diff" function that would work for all cases (e.g. when passed a tuple or a record of object from your state, as well as when passed a single object or a single primitive from your state). Perhaps there can't be a one-size-fits-all Elmish-specific memo builder?

vbfox commented 5 years ago

Was my original remark correct, then? In other words: Is it correct that lazyView doesn't "hide that fact" as you originally said, instead it just inserts one extra level of diffing?

It hide the "semantic" difference between these 2 different view of your app from react and force it to a full diff of it's children, whenever this diff will go one or 100 levels deep depends on the structure of the app. There is no technical hiding, only a conceptual one where the semantic structure of your page isn't transmitted to react.

And as said before if you can guarantee that it always go only one level deep (always a single custom semantically correct component under) then you are better off removing the lazyView and making this component pure.

Is this because lazyView serves the same purpose as PureComponent, which would be a better option than lazyView?

lazyView is a way to have a PureComponent with a syntax nearer to the rest of Elmish, memo is too conceptually even if being a React primitive it has some additional optimizations and isn't internally a Component class instance.

Yes, you're right. A suitable Elmish-specific memo builder could solve this, right? Hide all the diffing and prop construction and "just work" like lazyView does?

As you pointed same as memoBuilder it will need to be created outside of the view, but with a suitable syntax it's not much of a problem as it structure your code (As long as the extra syntax isn't too much)

For the diff I think the default should be a deep diff that ignore functions with a flag to go to a shallow diff (still ignoring functions) it would allow for example to pass a tuple with one or more parts of the state and a function and get good behavior by default.

If the shallow one would also handle tuples by default it would be perfect, ideally magically converting them to something like {"item0":"hello", "idem1":"world"} for nice viewing in the react debugger

cmeeren commented 5 years ago

Thanks! What kind of signature do you think the elmish memo builder should have? I have caught a heavy cold, so perhaps my brain isn't working correctly at the moment, but in order to not force the user to pack dispatch into a props object themselves, I imagine signature like this:

let elmishMemoBuilderWith
    (name: string)
    (equal: 'model -> 'model -> bool)
    (view: 'model -> Dispatch<'msg> -> ReactElement)
    : 'model -> Dispatch<'msg> -> ReactElement =

The usage would be the same as memoBuilder, except that render has signature 'model -> Dispatch<'msg> -> ReactElement instead of 'props -> ReactElement. However, I can't see how the above signature could work, because the returned value of the function has to be produced from a direct call to memoBuilderWith (not wrapped in a lambda that packs dispatch and model for you), and memoBuilderWith returns 'props -> ReactElement, so I can't see any way of packing/unpacking the dispatch on behalf of the user.

Is the best we can hope for simply a signature similar to memoBuilder with a good default diff?

vbfox commented 5 years ago

Yes the signature would look like this (except I think the With version should be an internal detail)

For me the real props (react side) would be a pure JS object without F# representation containing the members passed in model as well as a __dispatch member for dispatch. duirec

The implementation wouldn't really use memoBuilderWith but would be in JS and call React.memo directly

cmeeren commented 5 years ago

Ah, I see. I'll leave that to the experts. Thanks! :)