Skip to content
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

Should we make Fable check if a compatible object is passed to a function? #1986

Closed
MangelMaxime opened this issue Feb 27, 2020 · 12 comments
Closed

Comments

@MangelMaxime
Copy link
Member

@Zaid-Ajaj @ncave @forki @isaacabraham

In fable-compiler/fable-react#87, @njlr ask if it's possible to warn the user when he doesn't pass a record as the props for a react component.

My first answer was to say, that Fable doesn't allow that and people should use Fsharp Analyzer to add this feature to their project.

But now, I am wondering if we can't do something on Fable side. We all praise that Fable has great interop feature with JavaScript ecosystem, and I think React is not the only library that would want to have a record past as an argument to a function.

Is there already something in F# which could force the user to pass record?

If not, do you think we should add an attribute or something to tell Fable "check that the passed value is a record, anonymous record or an obj"?

obj too because people can use interop for creating their properties:

createObj [
    "propA", "valueA"
    "propB", "valueB"
]

It might look like this:

type [<AbstractClass; Import("Component", "react")>] Component<'P,'S>([<RecordOnly>] initProps: 'P) =
    [<Emit("$0.props")>]
    member __.props: 'P = initProps
    ...

type MyComponent(initialProps) =
    inherit React.Component<MyProps, MyState>(initialProps)

In the above code, Fable would know that MyProps should be a record, anonymous record or an obj.

If it's not the case, it will make the compilation fail and log a message to help the user fix the problem.

What do you think about this feature? Does it make sense and do you think the FCS gives enough info for doing it?

@ncave
Copy link
Collaborator

ncave commented Feb 27, 2020

@MangelMaxime Would any of the generic type constraints work instead?

@MangelMaxime
Copy link
Member Author

@ncave I was wondering the same thing, but I don't remember seeing any constraint available to test a record type.

@ncave
Copy link
Collaborator

ncave commented Feb 27, 2020

@MangelMaxime Records are reference types, so the : not struct constraint can help a bit, out of the box. There is also the base type or interface constraint, if props can be derived from a common class or implement a common interface. Or member constraint, if they have something in common in the member signature.

@njlr
Copy link
Contributor

njlr commented Feb 27, 2020

@MangelMaxime Records are reference types

Is this always true?

[<Struct>]
type Point = 
  {
    X : int
    Y : int
  }

Another complication is String, which is a reference type in .NET world but is treated like a value in JavaScript world.

@MangelMaxime
Copy link
Member Author

MangelMaxime commented Feb 27, 2020

Unfortunately : not struct is not enough because it's allow to pass a string

This code compile without error in the REPL

type Record =
    {
        FieldA : string
    }

type Component<'P when 'P : not struct>(props : 'P) =
    class end

let comp1 = Component({ FieldA = "" })
let comp2 = Component("")

@0x53A
Copy link
Contributor

0x53A commented Feb 27, 2020

Can I really only pass Records and not interfaces?

I'm also thinking about ts2fable generated bindings, where an interface might be better suited.

@ncave
Copy link
Collaborator

ncave commented Feb 27, 2020

Yes, that's kind of what I meant, obviously : not struct by itself doesn't get you all the way there, but it will filter out a good chunk of possible errors. But interfaces can be even more precise:

type IProperties = interface end

type Record =
    {
        FieldA : string
    }
    interface IProperties

type Component<'P when 'P :> IProperties>(props: 'P) =
    class end

let comp1 = Component({ FieldA = "" }) // works
let comp2 = Component("")     // compiler error

@0x53A
Copy link
Contributor

0x53A commented Feb 27, 2020

If you use an interface to tag it, you can't use anonymous records

@MangelMaxime
Copy link
Member Author

MangelMaxime commented Feb 27, 2020

You are right @0x53A I totally forget about the interfaces

By using an interface we can still use createObj, and jsOption<_>

open Fable.Core.JsInterop

type IProperties = interface end

type Record =
    {
        FieldA : string
    }
    interface IProperties

type Component<'P when 'P :> IProperties>(props: 'P) =
    class end

let createObjProps =
    createObj [
        "test" ==> "test"
    ]
    :?> IProperties

type IPropsContract =
    abstract PropA : string with get, set

let jsOptionsProps =
    jsOptions<IPropsContract>(fun o ->
        o.PropA <- "test"
    )
    :?> IProperties

// Fake a contract coming from an external JavaScript library
type IObject =
    abstract PropA : string with get, set
    inherit IProperties

let iObjectProps = unbox<IObject> null

let comp1 = Component({ FieldA = "" }) // works
let comp2 = Component(createObjProps)  // works
let comp2 = Component(jsOptionsProps)  // works
let comp2 = Component(iObjectProps)  // works
let comp2 = Component("")     // compiler error

But is it possible to use an anonymous record? 🤔

Edit: @0x53A was quicker than me :)

@ncave
Copy link
Collaborator

ncave commented Feb 28, 2020

Too bad anonimous records don't seem to implement IStructuralEquatable (perhaps not yet), unlike normal records:

open System.Collections

[<Struct>] type StructType = { FieldA : string }
type RecordType = { FieldB : string }
type UnionType = | FieldC of string

type Component<'P when 'P :> IStructuralEquatable>(props: 'P) =
    class end

let comp1 = Component({ FieldA = "" }) // works
let comp2 = Component({ FieldB = "" }) // works
let comp3 = Component(FieldC "")       // works
// let comp4 = Component({| FieldD = "" |}) // compiler error
// let comp5 = Component("")                // compiler error

@ncave
Copy link
Collaborator

ncave commented Feb 28, 2020

And using structural field subtypes doesn't help, even though it says the following here:

Anonymous types support both structural equality (if all constituent members support equality) and structural comparison (if all constituent types support comparison).

Anyway, wrapping it in a union works, but that's probably not what you're looking for:

open System.Collections

type UnionType = UnionType of {| Field1: string |}

type Component<'P when 'P :> IStructuralEquatable>(props: 'P) =
    class end

let comp1 = Component(UnionType {| Field1 = "" |}) // works

@MangelMaxime
Copy link
Member Author

MangelMaxime commented Feb 28, 2020

Indeed we can't wrap it in a DU.

In the end, it would probably need a complex code for something "simple". I mean, they have the problem in the JavaScript world too, people need to know that they can't pass a string as a prop but only an "object".

I will close this issue, but if someone has a clean solution we can re-discuss it.

Thank you, for helping to discuss the feature :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants