Issue 17 : Running Dash.NET from scripts

[ademar]

This PR executes the plan outlined on issue #17.

Mainly it moves server dependencies to its own projects Dash.NET.Giraffe and introduces Dash.NET.Suave that allows us to launch Dash applications under F# interactive and dotnet interactive. I've posted a more detailed description of the work under the issue.

Feedback is welcome.

[kMutagene]

I guess it is a quick workaround now, but this needs to be properly typed so that we can create DUs for Giraffe/Suave specific configs. Binding the first entry in an arbitrary string array is very confusing, at least it was for me when testing. I would suggest to add overloads for DashApp.run in Both Dash.NET.Giraffe and Dash.NET.Suave like this:

type DashSuaveConfig = { Port: int ; ...}
type DashGiraffeConfig = { Port: int ; ...}

type DashApp with

 static member run (args: string []) (gConfig: DashGiraffeConfig ) (dApp: DashApp) = ...
 static member run (args: string []) (sConfig: DashSuaveConfig ) (dApp: DashApp) = ...

[kMutagene]

It looks to me like the only thing we need separately in Dash.NET.Giraffe and Dash.NET.Suave eventually is the toWebPart / toHttpHandler (and run, see above) functions right? if so, we can keep all of the functions above in Dash.NET core and just add these as extension methods

[kMutagene]

Hey @ademar cool stuff overall!

moving the comments from #17 here:

The Suave app works fine as an application and a script (doing dotnet fsi Script.fs). It does not work in the Visual Studio 2019 F# interactive console as this is broken atm (see for example NuGet package references no longer work in F# Interactive (.NET Framework only) in VS 16.10.0 dotnet/fsharp#11622). They break this with every VS update.

It works for me when i use the Use .NET core scripting setting for FSI. Agreed though, it has happened several times that FSI breaks with VS updates =(

The Giraffe app does not work for me at the moment. We can see Kestrel booting up messages but it fails with "System.IO.IOException: Cannot determine the frame size or a corrupted frame was received."

I get that for http, but https works fine. Not sure where the exact misconfig is located though

I've used our own Suave.Html for the html bits in the Suave app. I realize I can probably use Feliz.Engine instead as it seems to be your intention going forward and it will minimize dependencies.

As we use Feliz.Rngine for components, it would be awesome if we could use it in both Dash.NET.Suave and Dash.NET.Giraffe

There is still some duplication of code we can eliminate.

See above basically, most of the duplication seems to be in the respective Views.fs file, if we can move that to Feliz.Engine, we can move it back into Dash.NET core and get rid of duplication.

As in my review comments, it looks to me like the only real difference is in DashApp.run and DashApp.toHttpHandler / DashApp.toWebPart

I've removed non Dash properties from the DashConfig record; wasn't sure on how to deal with it - I think we could just pass them in in the DashApp.run call.

Agreed on passing as args, but i would model the specific configs via types, see also my review comment

Overall i think this is ready to merge besides the backend specific config modeling, i would suggest we add that before merging as it can become very confusing using the string [] args.

Things like using Feliz.Engine can be added downstream, depends on how fast we want to package Dash.NET.Suave. (@jackparmer i suspect you want this out fast though? 😆)

[ademar]

Thanks! I'll take care of these today.

[jackparmer]

Things like using Feliz.Engine can be added downstream, depends on how fast we want to package Dash.NET.Suave. (@jackparmer i suspect you want this out fast though?

:)
Yeah, this is blocking @mr-m-coetzee from merging Installation/Getting Started docs for Dash.NET, so if this could be added in a follow-up PR, I'd prefer that!

[ademar]

Hi @kMutagene

I've made the configuration into typed records and have added the CORS headers to the Suave app.

I have also splitted the run method into run and runAsync. This could be useful in the future for the interactive visualization and allows to move the browser opening trick to the sample app.

Let me know what you think.

I spent some time trying to move the Views to the Feliz engine but it looks like it might take some time for me to wrap my head around it.

[kMutagene]

@ademar lgtm!

I realized that there is different behavior between Suave and Giraffe implementations regarding Json serialization:
I copied the code from the Dash.NET.Dev app into the suave script to check if callbacks are working. They do, but the dropdown component did not render. The problem lies here:

Dash.NET/src/Dash.NET/DashComponents/ComponentBase.fs

Lines 115 to 122 in 4c2559e

	type DropdownOption =
	{
	Label:IConvertible
	Value:IConvertible
	Disabled:bool
	Title:string
	}
	static member create label value disabled title = {Label=label; Value=value; Disabled=disabled; Title=title}

While these properties are uppercase in that record type, the dash renderer expect the Json properties to be lowercase:

This problem does not appear when using giraffe (which per default is also using Newtonsoft.Json, that's why I'm confused now). It is fixable by adding JsonPropertyAttributes to the record types, but i think it should be investigated why the json configs are different (@plt-joey worked a lot with components, maybe they have some insights here).

So in general I would say that this can be merged, but the serialization issue should be addressed separately before the release of a package (otherwise the suave app cannot render some components). Let's make sure to file an issue and use the exact same Json serializer options for both apps, or this will be a nightmare to maintain, especially if there might be more backend implementations in the future

[ademar]

@kMutagene

Giraffe uses camel casing serialization by default as seen here https://github.com/giraffe-fsharp/Giraffe/blob/master/src/Giraffe/Json.fs#L36

I've added the same settings in the last commit to this PR which solves the issue (ref #26 ).
I tested the dropdown menu renders now.

[kMutagene]

@ademar okay awesome, ill merge this now, thanks for the great work! 🎉