Add seq<ReactElement> overload to Html.div?

[cmeeren]

Add seq overload to Html.div?

from feliz.

[MangelMaxime]

Hello,

I can't find in the source where div accept a single ReactElement can you please point me to the source code?

What does the user code look like when doing so?

[cmeeren]

Feliz/src/Html.fs

Line 96 in ac24190

static member inline div (value: ReactElement) = Interop.reactElement "div" (createObj [ "children" ==> [| value |] ])

I'd guess user code looks like:

Html.div (
  Html.x [ ... ]
)

[MangelMaxime]

Ah yes, I missed this one thank you.

Personally, I would just implement Html.div (_ : ReactElement seq) because it would make the syntax consistent:

// Dummy type definition for Fable REPL
type ReactElement = class end

type IProp = interface end

type Html =
    static member inline div (value: ReactElement) : ReactElement = unbox ""
    static member inline div (value: ReactElement seq) : ReactElement = unbox ""
    static member inline div (value: IProp seq) : ReactElement= unbox ""
    static member inline div (value: string) : ReactElement= unbox ""

let xx =
    Html.div (
        Html.div [
            Html.div "Maxime"
            Html.div "Mangel"
        ]
    )

let yy = 
    Html.div [
        Html.div [
            Html.div "Maxime"
            Html.div "Mangel"
        ]
    ]

When having passing a single child we have to use (...) syntax so why not remove this one and always use [...] syntax.

[cmeeren]

Fine by me. I'm not sure I want to do the same in Feliz.MaterialUI because some component props explicitly require a single child component, but for a general div it might be the best choice.

[Zaid-Ajaj]

I have been thinking about this from the start but I have a couple of "problems" with this approach

First of all, using Html.div [ ] like this will not compile because F# cannot resolve which static overload to use: ReactElement list vs. IReactPropertly list and I think users expect this to "just work"

Second is when refactoring : if you start writing your code while prototyping with the following snippet

Html.div [
  Html.h1 "Fable"
  Html.h2 "React"
]

then you realize "Oh, I actually need to add a class to the div", suddenly you have to move the whole structure back to the following syntax

Html.div [
  prop.className "shiny"
  prop.children [
      Html.h1 "Fable"
      Html.h2 "React"
  ]
]

I think Html.div [...] syntax is nice for demos and examples but when it comes to using it in an actual application it is not really useful. Here I am assuming that the user almost always wants to add a class, key or other commonly used properties. Please let me know if you happen to use this pattern Html.div [ Html.div [ ... ] ] a lot in your applications because I would to add the such features on a commonly-used patterns basis.

The Html.div (...) is still nice for a couple of use-cases like Html.h1 (Html.strong "I am bold") but the arguments I stated above could be used against these as well so I am not really sure whether I want to add these overloads to the mix (except for Html.fragment because it is specifically useful when it takes a list of elements directly)

What do you guys think? @cmeeren @MangelMaxime

[cmeeren]

I agree that fragment should be used instead of div when you only want a list of children without props, and fragment already seems to have this overload:

Feliz/src/Html.fs

Line 11 in dc905aa

static member inline fragment xs = Fable.React.Helpers.fragment [] xs

So I'm happy. :)

[MangelMaxime]

First of all, using Html.div [ ] like this will not compile because F# cannot resolve which static overload to use: ReactElement list vs. IReactPropertly list and I think users expect this to "just work"

According to the REPL Fable is able to determine the correct overload:

Live version

type ReactElement = class end

type IProp = interface end

type Props =
    | Width of float
    | Height of float
    interface IProp

type Html =
    static member inline div (value: ReactElement) : ReactElement = unbox ""
    static member inline div (value: ReactElement seq) : ReactElement = unbox ""
    static member inline div (value: #IProp seq) : ReactElement= unbox ""
    static member inline div (value: string) : ReactElement= unbox ""

let xx =
    Html.div (
        Html.div [
            Html.div "Maxime"
            Html.div [
                Width 2.
            ]
        ]
    )

let yy = 
    Html.div [
        Html.div [
            Html.div "Maxime"
            Html.div [
                Width 2.
            ]
        ]
    ]

@Zaid-Ajaj Your point make sense, personnally I not a big fan of div (value: ReactElement) neither div (value: ReactElement seq) for the reasons you listed.

If I was to use Feliz (didn't had the time to test it for real yet), I think I would prefer to always use prop.children to keep consistent the code everywhere but perhaps it's just me :)

[cmeeren]

@MangelMaxime I think @Zaid-Ajaj specifically meant an empty list (literal []).

@Zaid-Ajaj For completeness: A trick you can use to help F#'s overload resolution (which I've used in several libraries) is to implement some members (often the more general/generic ones) as extension members (in an auto-opened module). For example, if you have an overload that takes 'a and another overload that takes 'a option, you'd implement the 'a member as an extension member. In this case, either of them could be an extension member and F# would pick the non-extension member when using []. It would "just work", as you say. But again, just mentioning this for completeness; it seems we all agree that fragment should be used if you need a container without props.

[MangelMaxime]

@MangelMaxime I think @Zaid-Ajaj specifically meant an empty list (literal []).

According to REPL it compiles too.

let yy = 
    Html.div [
        Html.div [
            Html.div "Maxime"
            Html.div [ ]
        ]
    ]

it seems we all agree that fragment should be used if you need a container without props.

Yes and no. fragment can have one prop which is key for optimisation but no other props.

[cmeeren]

According to REPL it compiles too.

Huh! Strange, I remember having problems with empty lists matching multiple overloads in other contexts. Truly, F# overload resolution worketh in mysterious ways.

[Zaid-Ajaj]

According to REPL it compiles too.

Yeah really weird, if I use the following, the compilation fails

type overloads =  
  static member inline func(xs: string list) = "string list"
  static member inline func(xs: int list) = "int list"

overloads.func [  ]
|> printfn "%s" 

but if I change string list into #seq<string> then it works but the compiler will make a choice for me

type overloads =  
  static member inline func(xs: #seq<string>) = "string list"
  static member inline func(xs: int list) = "int list"

overloads.func [  ]
|> printfn "%s" // int list

Weird stuff

[Zaid-Ajaj]

Again, I will keep this issue open until someone has better ideas, for now it seems that it won't help to have seq<ReactElement> overload because you almost always will use props and refactoring the code would be harder

[cmeeren]

Strange. In any case, I'm happy without such an overload, I just wanted to mention it. :)

[MangelMaxime]

The #seq<string> force the compiler to check if the given type can be down-casted into seq<string>, so I guess this new check is helping the compiler in this cases.

[Zaid-Ajaj]

Actually while thinking on this issue, I think that my assumption that you almost always need props is not really the case, especially for third-party libraries that already "include" classes and props in the default component implementation. Take reactstrap/Forms for example you have the snippet:

<Form>
  <FormGroup>
    <Label for="exampleEmail">Email</Label>
    <Input type="email" name="email" id="exampleEmail" placeholder="with a placeholder" />
  </FormGroup>
</Form>

Here, Form and FormGroup both do not have any props and they don't need any, so this would look something like this with Feliz

Form.form [
  FormGroup.formGroup [
     Label.label [ 
        label.htmlFor "exampleEmail"  
        label.text "Email"
     ]

     Input.input [ 
       input.email
       input.name "email"
       input.id "exampleEmail" 
       input.placeholder "with a placeholder"
     ]
  ] 
]

So for completeness sake, I will the seq<ReactElement> overloads and probably remove the single child overloads of only ReactElement as input