Freymaurer / Siren

A domain specific language (DSL) for creating mermaid graph YAML!
https://freymaurer.github.io/Siren/
MIT License
13 stars 0 forks source link

Investigate [<Optional>] attribute #20

Open Freymaurer opened 1 month ago

Freymaurer commented 1 month ago

Should improve C# accessibility, this would solve one of the two issues why a c# access layer is required!

A suggestion by Vladimir Shchur.

Seems to run for Fable.

REPL

Freymaurer commented 1 month ago

I tried it out and it works fine for c# with simple types such as integer and an existing default value.

Next i tried using Discrimate Union as parameter as replacement for f# option:

open Fable.Core

open System.Runtime.InteropServices

type ThatDiscriminatedUnion =
    | A of int
    | B of string

[<AttachMembersAttribute>]
type MyClass =
    static member Foo(req: int, [<Optional; DefaultParameterValue(null)>] opt1: ThatDiscriminatedUnion, [<Optional; DefaultParameterValue(null)>] opt2: ThatDiscriminatedUnion) =
        printfn $"{req} {opt1} {opt2}"
    static member Bar(args: struct(bool*string)[]) =
        for i in args do
            let struct (i1,i2) = i
            printfn $"{i1} {i2}"

module Test =

    MyClass.Foo(16, opt2=ThatDiscriminatedUnion.A) // does not compile
    MyClass.Foo(20)  // does not compile

    MyClass.Bar([| (true,"a"); (false,"b") |])

Plus fsharp intellisense looks anything but native:

image

Lanayx commented 4 weeks ago

@Freymaurer as for second issue, I recommended to get rid of tuples in API like here. If you use records, there (like { label: string; value: float }, this can both eliminate compatibility issue and make API more clear.

kMutagene commented 3 weeks ago

For reference, this is why we introduced Optional<T> for the native C# layer in Plotly.NET.CSharp

Using that type for optional values in the C# API makes C# API only slightly worse in intellisense (e.g. Optional<string> instead of string):

image

But you can remove all attributes from the F# function parameters, making them nice and legible again. This also supports both reference and value types therefore fixing both of your issues i think.

Note that you can just put the normal values in the C# call, no need to explicitly create Optional<T>:

image

Freymaurer commented 3 weeks ago

Ok, i did more investigation and i must say, optional types can most likely be replaced by using only [<Optional>], without DefaultParameter.

image

This is the base setup for Foo. Intellisense for using looks fine, just hovering over the function is not perfectly native for f#, but okay:

image

C# on the other hand looks really good, as it does not require any more setup and works beautifully (as far as i tested it):

image

What do you think @kMutagene ?

kMutagene commented 3 weeks ago

IIRC the problems start with generics and type constraints. E.g. If you try an arg with seq<#IConvertible> on the F# side and an equal type constraint in the C# wrapper method.

See the comments in the code i linked https://github.com/plotly/Plotly.NET/blob/dev/src%2FPlotly.NET.CSharp%2FOptional.cs#L10-L18

If you do not have these cases you should be fine with the standard optional param interop

Freymaurer commented 3 weeks ago

I tried again using [<Optional>] #seq<int> and here C# shows incompatibility, which makes using the api annoying:

image