fable-compiler / fable-react

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

yield and React.ofList in view function #136

Closed cmeeren closed 5 years ago

cmeeren commented 5 years ago

I have a question about usage of yield/yield! and toList. The latter is suggested in Fable: React you can be proud of ! Part 2: Optimizing React as a better alternative to yield!. For reference, the implementation is:

https://github.com/fable-compiler/fable-react/blob/0a0bd8821e738070857ea81219ab0620a89d3e1c/src/Fable.React/Fable.Helpers.React.fs#L1002-L1003

It says from .render() method. Does this mean that this should not be used from an Elmish view function? It seems to work fine, but I could be missing broken behavior in important edge cases.


In general: Is it always dangerous to use yield/yield!? The linked article is a bit unclear IMHO, since it says that yield! would actually work in that case. I am using yield to conditionally insert props, for example:

let view model dispatch =
  p [ if model.MessageImportant then yield Class "important" ] [ str "Message" ]

I also use it to support match and if expressions for (non-list) child elements, for example:

let view model dispatch =
  main [] [
    yield
      match model.CurrentPage with
      | Home -> HomePage.view model dispatch
      | About -> AboutPage.view model dispatch
  ]

Another example:

let view model dispatch =
  div [] [
    yield button [ OnClick (fun _ -> Dispatch FetchData) ]  [ str "Fetch data" ]
    if model.IsFetchingData then yield progressIndicator []
  ]

Are any of these uses of yield suboptimal? If so, how could they be improved?

MangelMaxime commented 5 years ago

@vbfox made a talk about this subject at FableConf 2018.

You can see it here.

Short answer, is:

cmeeren commented 5 years ago

Great, thanks! What about ofList? Can it be used from the Elmish view function, or only directly inside a component's render method?

vbfox commented 5 years ago

Also except if your list change often (especially with insertions) using a good page structure with memo components for each big part will give better perf gain than micro optimizing your lists (So if you plan on doing only one of the 2...).

cmeeren commented 5 years ago

Thanks! Great answer. The only thing I'm a bit unclear on is your mention of "memo". Are you talking about memoization, or (Elmish) lazy views, or something else? Your description seems to fit Elmish lazy views, but the word "memo" confuses me a bit in that respect.

vbfox commented 5 years ago

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.

cmeeren commented 5 years ago

Thanks for the clarification! I didn't know about memo. Perhaps I'll try it out. I also watched your talk @MangelMaxime linked to, which was very clarifying.

I see that memo is only available in Fable-React 5.0.0-alpha. Is there a roadmap anywhere? I'm considering experimenting with it, and it would be great to know which parts of the API is likely to change until the final 5.0.0 release so my codebase doesn't grow too attached to them. 😅

MangelMaxime commented 5 years ago

I don't think there is much breaking changes to expect. At the time, we were more or less waiting for react ecosystem to catch on memo support before releasing it.

cmeeren commented 5 years ago

Great! But I'm curious: Why is it relevant

waiting for react ecosystem to catch on memo support

before releasing Fable.React 5.0.0?

alfonsogarciacaro commented 5 years ago

Actually I was waiting to see if React hooks become stable in case we have to make some other (minor) breaking changes to support them. I'm also considering to remove most of the bindings originally generated with ts2fable (Fable.Import.React.fs) and only leave the manual bindings (Fable.Helpers.React.fs) which are more idiomatic in F#.

The memo helpers are unlikely to change as they're working well and the API is already stable in React.

cmeeren commented 5 years ago

While I don't know the details, a general big 👍 from me on removing unneeded auto-generated bindings. I've found the immense number of (seemingly auto-generated, non-F#-idiomatic) types to be a considerable source of confusion when trying to learn Fable and Fable.React (I'm still deep in the learning process).

In any case, I think my questions here are answered, so I'll close. Thanks! :)