glutinum-org / cli

https://glutinum.net/
59 stars 6 forks source link

Consider using `delegate` to represents `FunctionType` #143

Closed MangelMaxime closed 3 weeks ago

MangelMaxime commented 3 weeks ago

History of the discussion available here #135

By using delegate, we keep the parameter names information.

Need to check, if by using delegate we can still set the property on the interface. And what impact it has globally, perhaps we can also sometimes use delegates and sometimes flat functions.

MangelMaxime commented 3 weeks ago

I am taking a look at this issue and it seems like FunctionType is only used inside of interface or class methods in TypeScript.

So I am looking to make the F# output friendlier by using delegates.

My current idea, is if there is 0 or 1 parameters then I am generating a lambda.

Otherwise, it will generate a delegate.

The reason for that is I think for 1 parameter having the name of the parameter don't add a lot of value compared to the noise in the user output. Like with everything, it can be subject to change based on UX and feedbacks overtime.

I wanted to explains the reasoning to keep a track and in case someone as another vision.

cc @roboz0r (because we had a small chat about it on Discord)

roboz0r commented 3 weeks ago

Seems like the only major downside to delegate is the user must call the function using .Invoke. If generating the code we could also generate extensions that allow you to call the method naturally. inline means there's no extra js in the output.

type Callback = delegate of title:string * message:string option -> unit

type Delegate =
    abstract member alert: Callback with get,set

type Delegate with 
    member inline this.Alert(title:string, message:string option) = 
        this.alert.Invoke(title, message)

let callDelegate(a: Delegate) = a.Alert(title = "a", message = None)
export function callDelegate(a) {
    a.alert("a", undefined);
}
MangelMaxime commented 3 weeks ago

That's an interesting idea, I opened a new issue to keep track of this idea and to list the impact/drawback it can have.

See #144

roboz0r commented 3 weeks ago

Not sure if this belongs here or in #144 (or a new issue) but I tried hacking on the ParamObject syntax and came up with the following that avoids extension members but keeps the need to change case or other minor name change:

open Fable.Core

type Callback = delegate of title:string * message:string option -> unit

[<AllowNullLiteral>]
[<Global>]
type Delegate
    [<ParamObject; Emit("$0")>]
    (alert: Callback) =
    member val alert: Callback = jsNative with get, set

    [<CompiledName("alert")>]
    member inline this.Alert(title, message) = this.alert.Invoke(title, message)

let d = Delegate(fun title message -> printfn "%s %A" title message)
let callDelegate(a: Delegate) = a.Alert("a", None)

callDelegate d
d.Alert("a", None)
d.alert <- (fun title message -> printfn "changed %s %A" title message)
d.Alert("a", None)
MangelMaxime commented 3 weeks ago

What does CompiledName do ? It was not required before for the same code.

I don't think this new code can be used, because it is using a class and F# classes don't support multi inheritance.

roboz0r commented 3 weeks ago

Without CompiledName the method call still comes out as d.Alert in the js instead of d.alert

roboz0r commented 3 weeks ago

You can include an interface without changing the output JS:

[<AllowNullLiteral>]
type IDelegate =
    abstract member alert: Callback with get, set

[<AllowNullLiteral>]
[<Global>]
type Delegate
    [<ParamObject; Emit("$0")>]
    (alert: Callback) =
    member val alert: Callback = jsNative with get, set

    [<CompiledName("alert")>]
    member inline this.Alert(title, message) = this.alert.Invoke(title, message)

    interface IDelegate with
        member val alert: Callback = jsNative with get, set
MangelMaxime commented 3 weeks ago

A first implementation with delegate has been done, the rest is tracked in the new issue.