Closed object closed 2 months ago
Actually there is a need for both old and new helper functions. Checked my code, there is still use of old definition:
let inline asSubscription (this: BridgeConfig<_, _>) dispatch = this.Attach dispatch
But in application setup helper code needs to look differently, like my PR (I used name "addSubscription"):
let inline addSubscription (this: BridgeConfig<_, _>) =
let sub dispatch =
this.Attach dispatch
{ new System.IDisposable with
member _.Dispose() = () }
[ [ "Elmish"; "Bridge" ], sub ]
I.e. the old helper function can be kept, but it would be useful to define a helper above. What do you think?
Hi there! Apologies for the delay. That sounds like a great idea. I'll go ahead and implement those changes. The BridgeConfig<_,_>
type already implements System.IDisposable
, we can return this
on the sub
function. Additionally, we should include the BridgeConfig<_,_>.name
argument in the subscription to have better control over multiple connections.
I checked it again, the Program
helpers are already migrated to the new format. We can probably make that code a helper to be shared with the path where we are using it with Cmd.ofEffect
.
EDIT: I saw your code on #59 and noticed the missing scenario. We have a BridgeConfig<_,_>
that depends on the model. We could use the Bridge.asSubscription
on the Cmd.ofEffect
in the init
function, but that is not ideal. I will create a model-dependent subscription function to support that.
I added an extra commit to your fork, can you check if it works as intended? I'm at a computer without any projects using Bridge, so I only type-checked the changes without real testing.
For your original code, you have two options:
let bridgeSubscription (state: State) =
Bridge.endpoint state.AppSettings.SocketUrl
|> Bridge.withUrlMode Raw
|> Bridge.withMapping (fun x -> x |> MediaSetEvent)
|> Bridge.asSubscription
Program.mkProgram init update view
|> Program.withSubscription (bridgeSubscription >> Cmd.ofSub)
|> Program.withReactSynchronous "elmish-app"
|> Program.withDebugger
|> Program.run
Using the configurator (recommended):
let bridgeConfigurator (state: State) =
Bridge.endpoint state.AppSettings.SocketUrl
|> Bridge.withUrlMode Raw
|> Bridge.withMapping (fun x -> x |> MediaSetEvent)
Program.mkProgram init update view
|> Program.withBridgeConfigurator bridgeConfigurator
|> Program.withReactSynchronous "elmish-app"
|> Program.withDebugger
|> Program.run
Or using it on init:
let bridgeInit initArg =
let (state:State, cmds) = init initArgs
let effect =
Bridge.endpoint state.AppSettings.SocketUrl
|> Bridge.withUrlMode Raw
|> Bridge.withMapping (fun x -> x |> MediaSetEvent)
|> Bridge.asSubscription
state, effect::cmds
Program.mkProgram bridgeInit update view
|> Program.withReactSynchronous "elmish-app"
|> Program.withDebugger
|> Program.run
@Nhowka thank you for the update. I tried to apply it to my code.
At first, the code doesn't compile because of "option":
Program.mkProgram init update view
|> Program.withBridgeConfigurator bridgeConfigurator
The signature of bridgeConfigurator is (config: -> BridgeConfig<,> option) (program : Program<, , , _>). And "brigdeConfigurator" returns BridgeConfig<ClientMsg,Msg>. So to make it compile I had to change the application setup code like this:
Program.mkProgram init update view
|> Program.withBridgeConfigurator (bridgeConfigurator >> Some)
So if the use of bridgeConfigurator with "Some" is intended, then it can stay like you proposed.
Cheers!
So if the use of bridgeConfigurator with "Some" is intended, then it can stay like you proposed.
That's right, missed that! Version 7.1.0 of the clients were uploaded to NuGet, soon they should be up! Thanks for catching that missing feature, soon I will take some time to update the readme
This PR updates asSubscription helper method for changes in Elmish v4.
Elmish subscriptions have been revised in v.4, and there is a migration guide: https://elmish.github.io/elmish/docs/subscription.html#migrating-from-v3
NB! asSubscription helper method is also defined in DotNetClient library, but I am note sure if it needs a similar change. @Nhowka Please check if DotNetLibrary method needs an update.