Add tests for using ParamObject constructor overload
Description
Recently, I discovered that we could use constructor overload in order to improve the binding experience when using F#. In the following, exemple we are able to abstract away the U2<string, ResizeArray<string>> from the user and he can just directly provide a string or ResizeArray<string> making the code written looks like standard F# code.
[<AllowNullLiteral>]
[<Global>]
type FuseOptionKeyObject [<ParamObject; Emit("$0")>]
private (name: U2<string, ResizeArray<string>>, weight: float) =
[<ParamObject; Emit("$0")>]
new (name: string, weight: float) =
FuseOptionKeyObject(U2.Case1 name, weight)
[<ParamObject; Emit("$0")>]
new (name: ResizeArray<string>, weight: float) =
FuseOptionKeyObject(U2.Case2 name, weight)
member val name: U2<string, ResizeArray<string>> = jsNative with get, set
member val weight: float = jsNative with get, set
let fuseOptionString =
FuseOptionKeyObject("name", 1)
let fuseOptionResizeArray =
FuseOptionKeyObject(ResizeArray [ "name" ], 1)
generates
export const fuseOptionString = {
name: "name",
weight: 1,
};
export const fuseOptionResizeArray = {
name: ["name"],
weight: 1,
};
However, if we use constructor overload with only a subset of the arguments then only the directly provided arguments are used:
[<Global>]
type Circle [<ParamObject; Emit "$0">] private (kind: string, radius: float) =
[<ParamObject; Emit("$0")>]
new (radius : float) =
Circle("circle", radius)
member val kind: string = jsNative with get, set
member val radius : float = jsNative with get, set
let circle = Circle(radius = 4.2)
generates
export const circle = {
radius: 4.2,
};
My expectation would be to have:
export const circle = {
kind: "circle",
radius: 4.2,
};
I think I understand why we have the current behaviour ParamObject is probably just looking at the argument used in the constructor and not the actual code but it would nice if we could have the "expected behaviour" that I described. This could improve situation like the example shown in https://github.com/fable-compiler/repl/issues/150#issuecomment-1095152633
While we are at it, I think it could be nice if we could write the binding like that:
[<Global>]
type Circle [<ParamObject; Emit "$0">] private (kind: string, radius: float) =
new (radius : float) =
Circle("circle", radius)
member val kind: string = jsNative with get, set
member val radius : float = jsNative with get, set
let circle = Circle(radius = 4.2)
Note how I removed the [<ParamObject; Emit "$0">] from the constructor overload. This is just to try simplify a bit the syntax but I am not sure if this is a good idea or not.
Related information
- Fable version: 3.7.9
- Operating system: REPL
- REPL: 3.7.9.1
I'd have to think about your proposal, but your particular use case should be possible already using the spread operator:
[<Global>]
type Circle [<ParamObject; Emit "{ kind: \"circle\", ...$0 }">] (radius: float) =
member val radius: float = jsNative with get, set
let a = Circle (radius = 1.)
This is the generated code:
export const a = { kind: "circle", ...{
radius: 1,
} };
Indeed, I didn't think about adding the code in the Emit instruction.
Sorry for the late reply, for some reason I thought the discussion was settled 😅 TBH, I think we're already stretching the original goal of ParamObject/NamedParams attribute too far. I'm not sure whether bindings like this are a good option: they require you to "fake" the member val properties and also use Global attribute because Erase is not working well. And also I'd like to discourage as much as possible the use of secondary constructors because they're a nightmare to compile to languages that don't support them (I'd like to forbid them for Dart compilation). They are the reason we need to wrap all class constructors in Fable even if secondary constructors are not so common in F#.
It's only slightly more verbose but for this kind of "typed pojos" I would use Fable's traditional way of doing bindings, that is, interfaces, and provide static methods as extensions for construction. As in:
open Fable.Core
open Fable.Core.JsInterop
type Circle =
abstract kind: string with get, set
abstract radius : float with get, set
[<AutoOpen>]
module Extensions =
type Circle with
static member inline Create(radius: float, ?kind: string): Circle =
!!createObj [
"radius" ==> radius
"kind" ==> defaultArg kind "circle"
]
let circle = Circle.Create(radius = 4.2)
There are a couple of optimizations that we can do in this code: we can make createObj ignore fields if the value is None, and also try to resolve defaultArg at compile time (use directly the alternative if value is None).
Disclaimer: I had a hard to putting in word my though so it is possible that something is not clear. If needed, don't hesite to discuss it further.
And also I'd like to discourage as much as possible the use of secondary constructors because they're a nightmare to compile to languages that don't support them (I'd like to forbid them for Dart compilation)
@alfonsogarciacaro I think you are messing up 2 things in your answer the binding feature and the F# features supported by Fable when generating code.
The feature being discussed in this issue is about bindings, we are not actually generating several constructors.
When consuming bindings with ParamObject constructor we don't create 2 constructors, we generate a POJO directly. Which is something really come in JavaScript, I can't speak for other languages like Dart.
The goal of this feature is to provide a good F# syntax for a features that is supported by the target language but not by F#.
If you think that this proposition:
[<Global>]
type Circle [<ParamObject; Emit "$0">] private (kind: string, radius: float) =
new (radius : float) =
Circle("circle", radius)
member val kind: string = jsNative with get, set
member val radius : float = jsNative with get, set
let circle = Circle(radius = 4.2)
is not going to be accepted it is unfortunate because this would allow lighter binding code. But we can live without it.
The only thing that I want to make sure, is that the overload syntax keep working in future version of Fable because I am basing my bindings on it now. The reason I am using it now, is because this provide the best user experience when creating POJO now.
I will send a test suite to make sure this is still supported in the future
About what F# features to support when generating code.
It is important that Fable keeps supporting all of F# features so F# users can use the language they know as they know it. If you don't, then their will be a lot of extra knowledge to have before writting F# code throught Fable which is not good.
For me one of the reason why Fable is working really well in F# community is because, when people write "any" F# code it just works. It is true that Fable doesn't support the whole BCL, reflexion API and spans (memory acces) but other than that everything works.
This allow users to share code between their server and client, re-using their existing knowledge for free.
If you were to not support some of the F# syntax, IHMO this is bad because the IDE will say that it is working but Fable say no. Resulting in a poor user experience.
JavaScript doesn't support constructor or method overload neither, but Fable still support it and this is important for some libraries.
Dark doesn't support method overload neither. So if we were to follow what you say, Fable should not support them neither and so this means that you would not be able to use object oriented code anymore or only a subset of it which can be too limited for F# developers.
@MangelMaxime I guess I concentrated too much on the proposed code snippet instead of asking the right questions 😅 For your Circle use case, can you use TypeScript-style tagged unions? They're supported since #2618. It's basically what @alfonsogarciacaro suggests, but a bit safer because you don't have to use createObj.
@alfonsogarciacaro I think constructor overloads are a good fit for bindings, at least for JS/TS libraries. Otherwise authors would have to emulate them with U{N}. As a .NET developer, I think it makes reading signatures much easier. For this reason, I really dislike reading TypeScript signatures, when you have to wade through a monstrosity like this:
function f(
a: ALongTypeName | AnEventLongerTypeName | SomeContainer<IsThisAGenericTypeParameterOrWhat>,
b: YouGetMyPoint | undefined)
: Result | undefined
@inosik I didn't have the chance yet to need TypeScript-style tagged unions.
I think the circle case was just an example of ony of the benefit I had in mind in the past of not repeating ParamObject on contructor overloads. And to emit TypeScript-style tagged unions types but using DUs in F# is probably a better solution here as we can benefit from the compiler checks.
The main point of this issue was about not repeating ParamObject to have a lighter syntax. I don't know if that's the right thing to do or not. As I said, this is not something blocking to have to repeat ParamObject attributes just a bit heavy ^^
Many things here, I'll try to reply later but just to point out quickly that I remembered we already have syntax to deal with optional arguments in Emit :) Currently it only accepts boolean literals but with an easy fix I made locally we can have code like this (note I've removed ParamObject here):
[<Global>]
type Circle [<Emit """{
radius: $0,
kind: {{ $1 ? $1 : 'circle' }}
}""">] (radius: float, ?kind: string) =
member val kind: string = jsNative with get, set
member val radius : float = jsNative with get, set
let circle = Circle(radius = 4.2) // { radius: 4.2, kind: 'circle' }
let circle = Circle(radius = 4.2, kind = "foo") // { radius: 4.2, kind: 'foo' }
To be clear, my reluctance to introduce more compiler machinery here is the original example already has 3 special Fable attributes modifying the type/constructor behavior: Global, ParamObject, Emit. Adding more particular cases instead of reusing the current possibilities (with some tuning for performance and consistency) increments the combination matrix which can add confusion to both library authors and users.
You are focusing on the Circle example but the goal of this example was just to say that when the main constructor is docrated with ParamObject that behaviour is not forwarded to the overloaded constructor, forcing the user to repeat ParamObject.
The original goals of this issue was to share:
- The discovery I made with constructor overload
- an idea I had to make a lighter syntax possible.
Let's say, that the first point has been a success and the second is not :)
From my POV the action that this issue required now is to make sure that this behaviour is tested to avoid regression in future release.
Hmm, besides the technical difficulties I'm not sure ParamObject should be automatically applied to a secondary constructor. Conceptually the attribute is similar to ParamArray (thus the name). In "standard" F#, If you have ParamArray in the main constructor you don't expect it to be extended automatically to other constructors.
In any case, I absolutely agree we need more tests to make sure new version don't break bindings :) There're a few here but any more you can add will be very welcome! https://github.com/fable-compiler/Fable/blob/8689186691ab4529c39b445ca85bc1b9fd20ddbb/tests/Main/ImportTests.fs#L113-L124
BTW, ParamObject is currently broken for functions declared in F# as it was mainly intended for bindings. I'm planning to fix it by I prefer to do it in Fable 4 because I've updated the AST to include more information about "named params". The resolution of ParamObject in Fable 3 is a bit messy at the moment.
In any case, I absolutely agree we need more tests to make sure new version don't break bindings :) There're a few here but any more you can add will be very welcome!
Will do.
BTW, ParamObject is currently broken for functions declared in F# as it was mainly intended for bindings.
In my mind, it was always about to be used in bindings and using members.