-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Investigate [<Optional>] attribute #20
Comments
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: |
@Freymaurer as for second issue, I recommended to get rid of tuples in API like here. If you use records, there (like |
For reference, this is why we introduced Using that type for optional values in the C# API makes C# API only slightly worse in intellisense (e.g. 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 |
Ok, i did more investigation and i must say, optional types can most likely be replaced by using only This is the base setup for 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): What do you think @kMutagene ? |
IIRC the problems start with generics and type constraints. E.g. If you try an arg with 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 |
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
The text was updated successfully, but these errors were encountered: