Optional component record fields

[mr-m-coetzee]

Current component record implementations does not implement optional fields as optional.

• This force long winded code.
• Puts it on the user to specify a default value for optional fields.
• Is inconsistent with implementations for Python, Julia, R and MATLAB (as far as I can tell).
• Not clear from the code which fields are optional.

Current state

Taking DropdownOption as an example.

The fields are described as:

• Label: string | number (required)
• Value: string | number (required)
• Disabled: boolean (optional)
• Title: string (optional)

But is implemented as:

type DropdownOption = 
    {
        Label:IConvertible
        Value:IConvertible
        Disabled:bool    // Optional - but must provide a value
        Title:string    // Optional - but must provide a value
    }
    // Both optional arguments require a value
    static member create label value disabled title = {Label=label; Value=value; Disabled=disabled; Title=title}

Usage:

Dropdown.Attr.options [
    // Forced to provide a default value
    DropdownOption.create "Population" "pop" false "Population"
    DropdownOption.create "Life Expectancy" "lifeExp" false "Life Expectancy"
    DropdownOption.create "GDP per Capita" "gdpPercap" false "GDP per Capita"
]

Proposal

1. Make optional fields optional.

   type DropdownOption = 
   {
       Label: IConvertible
       Value: IConvertible
       Disabled: bool option    // Clearly optional
       Title: string option    // Clearly optional
   }

2. create method making use of tuples with optional arguments (also allows overloading where curry does not).

   static member create (label, value, ?disabled, ?title) = { Label=label; Value=value; Disabled=disabled; Title=title }

Usage:

Dropdown.Attr.options [
    DropdownOption.create ("Population", "pop")
    DropdownOption.create ("Life Expectancy", "lifeExp", title = "My Life Expectancy")
    DropdownOption.create ("GDP per Capita", "gdpPercap", true)
]

Example convert method:

static member convert this =
    box {|
        label = this.Label
        value = this.Value
        disabled = match this.Disabled with Some isDisabled -> box isDisabled | None -> null
        title = match this.Title with Some title -> box title | None -> null
    |}

[kMutagene]

This is why DynamicObj is used in this library, it enables the init/style pattern (see here for an example where we create an object in plotly.net). It is a powerful pattern that also allows way better C# interop than the record approach (also, you do not have to handle None for serialization, and serialization overall works like a charm).

I agree that these records should be switched to use optional fields, but i would recommend to use the same pattern we use across the plotly libs to create these json objects with optional fields. The pattern (in a slightly different/adapted form) is already used for components, see an example here.

It is definitely more verbose, but we have many clear advantages. Conversion of your example here would be:

type DropdownOption() =
    inherit DynamicObj()

    static member init
        (
            label: IConvertible,
            value: IConvertible,
            ?Disabled: bool,
            ?Title: bool
        ) =
            DropdownOption()
            |> DropdownOption.style(
                label,value,
                ?Disabled = Disabled,
                ?Title = Title
            )

    static member style 
         (
            label: IConvertible,
            value: IConvertible,
            ?Disabled: bool,
            ?Title: bool
        ) = 
            fun (dro:DropdownOption) ->

                label   |> DynObj.setValue dro "label" // this is where you decide the property names for json in this pattern
                value   |> DynObj.setValue dro "value"
                Disabled|> DynObj.setValueOpt dro "disabled"
                Title   |> DynObj.setValueOpt dro "title"

                dro

[kMutagene]

You might even just go for init and not using style at all, as style is usefull if you have a DropdownOption for which you want to set/change properties - something that most likely never happens, at least for this kind of object.

[mr-m-coetzee]

So there is already a good solution using DynamicObj.

Why do we use records then, is there a plan to replaced them with DynamicObj?

[kMutagene]

Well I would say I overlooked that while creating the POC. If there are optional fields, the DynamicObj approach is the way to go.

[mr-m-coetzee]

Okay great.

So we can leave this issue open then with the plan of replacing records with DynamicObj?

I can have a look at it.

[kMutagene]

Just for clarification, i would suggest that we only apply that style for objects that have optional fields. Things such as DashApp, where everything is mandatory should stay records with JsonProperty attributes (where needed)

[kMutagene]

Also, i added some tests for serialization of component prop types (which, by design, fail at the moment) https://github.com/plotly/Dash.NET/commit/9090c1ad86b03a4087bc13310187d28ff45d872e. I am not sure which of these records have optional fields, but i would suggest starting there (removing the convert functions, using either JsonProperty or DynObj to make tests green). Would also be awesome if we could set up creation of tests together with component auto generation, but thats another issue.

[mr-m-coetzee]

Also, i added some tests for serialization of component prop types (which, by design, fail at the moment) 9090c1a. I am not sure which of these records have optional fields, but i would suggest starting there (removing the convert functions, using either JsonProperty or DynObj to make tests green)

Great - Thanks

[mr-m-coetzee]

:point_up: @kMutagene, with the tests you added - how will PR's unrelated to this pass if they're failing already?

[kMutagene]

@mr-m-coetzee I added the necessary changes to make the tests work in #33 , just need quick feedback from @plt-joey about the changes to the component generation project