Implement Elmish Commads in ReactiveCom
Fix #62. Enables Elmish Commands in ReactiveCom:
- Added a depedency to Fable.Elmish
- Implemented Cmd<'Msg>
- Implemented an error handler in the component
Great work @EdouardM, thank you! Hmm... I'd prefer to avoid the dependency as Fable.React should be architecture agnostic. Maybe move reactiveCom to Elmish.React (though @MangelMaxime didn't like it very much :wink:) or to its own package?
Any thoughts on this @zaaack?
At the very beginning I implemented Reactive Component only for ui components which only contains small UI state that I don't want to put in global state, so Elmish style side effects manager is not needed here.
But if you want to do side effects like fetch some data from network, currently you can only do it in view function because only view has access to dispatch. Perhaps we can passing dispatch to the update function or even further, moving dispatch to component instance method, and passing the instance to update as context, then we can access all react functionality in update, which is much light weight and don't need to dependent on Elmish.
I agree that Fable.React should not include a deps on Fable.Elmish
I am much more in favor of an independant package but I am probably in minority here :)
I agree I can send a PR to Elmish.React with this something similar to this code.
I could also add another "simple" Reactive Component with no side effect manager. Like it is the case in Fable.Elmish with mkSimple and mkProgram.
So these are our options 😄
- Move
reactiveComto Fable.Elmish.React to use ElmishCmd - Do 1 but also keep a simple
reactiveComwithout side effects in this library - Pass the
dispatchfunction to update (or movedispatchto component instance method) as @zaaack suggests
Not sure what's the best one, though I'd try to avoid confusion for the users (for example, having two similar components in different libraries). Passing the current instance as context will probably mean we go "full React" and reactiveCom becomes just a way to build a reusable React component with a nicer syntax in F# 🤔
There is another option, mixing 1 and 2: Do 1 and also have a simple reactiveCom without side effects in the Elmish.React library.
I'd vote for option 2, giving them separate names, eg reactiveElmishCom
@et1975 What do you think? What would be your preferred way to create reusable React components using the Elmish architecture?
I agree that React bindings should remain just that - language support for the library. However tempting tacking more helpers on it might be, it would impact maintainers ability to keep up with changes in React and Fable.
I also don't see adding any stateful components support into Elmish.React happening, as me and my colleagues have managed to avoid needing it over the course of several years and numerous projects... maybe we just didn't have the problem you are facing or maybe there's a stateless solution that would accomplish it.
In any case, I'm in the Separate Library camp. Even if it's just 1 F# file, one could import it as github dependency with paket.
So what do we do? Shall we move this to an independent package (we can put it in this repo if you prefer) with the Elmish dependency? And remove reactiveCom from here or make it more React-ish?
I think it would be possible to make 'Cmd a generic parameter to avoid the dependency on Elmish. But I guess this is stale.
Yes, we haven't dealt with this PR for a very long time 😅 But you're right @Byte-666, that could be a solution. I've been also thinking about it and if we want to create React components in Elmish-style, instead of bringing the full Elmish architecture here, maybe we should just PR to Elmish.React to add the possibility of instantiating the Elmish app as a component instead of a full program.
Yeah, I did something like that, creating a full elmish style rooted component.
I was trying to solve that other performance problem with MaterialUI withStyles, the problem was elsewhere (makeStyles solved it, together with the solution to that pesky InputRef problem, using Elmish.React.Helpers.valueOrDefault [already closed] ).
Example:
[<RequireQualifiedAccess>]
module Internal =
type RootProps<'model> = {
key: string
init: unit -> 'model
hook: ('model -> unit) -> unit
equal: 'model -> 'model -> bool
render: 'model -> ReactElement
}
type RootView<'model> (props) as this =
inherit Component<RootProps<'model>, 'model> (props)
do this.setInitState <| props.init ()
do props.hook <| this.update
member this.update model = this.setState ( fun _ _ -> model )
override this.shouldComponentUpdate (_nextProps, nextState) =
not <| this.props.equal this.state nextState
override this.render () =
this.props.render this.state
let rootView2With key equal init hook render =
ofType<RootView<_>,_,_>
{ key = key; init = init; hook = hook; equal = equal; render = render }
[]
let withReactRootUsing equal placeholderId (program: Elmish.Program<_,_,_,_>) =
let setState = ref ignore
let root =
Internal.rootView2With "root"
( fun x y ->
match x, y with
| (Some x, _), (Some y, _) -> equal x y
| _ -> false )
( fun () -> ( None, ignore ) )
( fun hook -> setState := hook )
( fun ( model, dispatch ) ->
match model with
| Some m -> Program.view program m dispatch
| _ -> div [] [] )
let renderRoot () =
ReactDom.render ( root, document.getElementById placeholderId )
let setState model dispatch =
let render = lazy ( renderRoot () )
!setState ( Some model, dispatch )
render.Value
program |> Program.withSetState setState
I plugged it to Elmish, but you could supply setState to any kind of dispatcher and it would work.
Set state is not fired from the render, that makes it easier to plug into Elmish dispatcher, and I don't think it's a problem for Stateful React components.
But I agree that this code like this should belong elsewhere, not here, and this is only binding and React has tons of ways of doing things.
Now we could even use Hooks to create this kind of feature. (I'll try it later as I don't like creating OO types, also I didn't like using ref, but it made the code shorter by using Elmish infrastructure)
If it's ok with everybody I will close this PR as the preferred way to have Elmish components now is useElmish: https://github.com/Zaid-Ajaj/Feliz/issues/481