How to spread existing object props along with Feliz props?

[zaid-ajaj]

How to spread existing object props along with Feliz props?

[cmeeren]

Oh, all the trickery! 😵 Thanks a lot! You're a Fable wizard! 🧙‍♂️

[Zaid-Ajaj]

Not very short and used in a number of places => do not inline. if it used once or twice in your app to call a couple of functions inline, no problem at all.

[Zaid-Ajaj]

Are there better ways to accomplish this?

Not that I know of, I am glad you already found a workaround

Since this is completely general and might be needed in many situations, would you consider adding support for this in Feliz?

Actually I think this usage of getLabelProps() is too low-level for a Fable binding, the idea of a spread is a Javascript thing and when building Fable bindings, I believe that one should keep F#-y things only, so if you are building a binding for downshift, I would expect the binding to give me:

getLabelProps : unit -> IReactProperty list

which wraps the internal getLabelProps with objectEntries in the binding implementation (i.e. getLabelProps >> objectEntries) then I could yield! with the other properties:

Html.div [
  yield! (getLabelProps())
  yield prop.className "foo"
]

I guess the same applies for spreading a style

[cmeeren]

I see your point, I agree in the context of getLabelProps (I have written bindings for downshift and know it's not hard to do there).

But what about MUI's mixin? It's defined some levels down (theme.mixins.toolbar) in interfaces that are only used for directly accessing native JS code, as is commonly seen in bindings. Changing this to return something else would mean that everything up the hierarchy would also need to be modified to not be interfaces but instead classes with custom logic. It seems like this can, in general, cause a potentially big maintenance step up from (slightly tweaked) ts2fable bindings, just to change this one thing. And this would be necessary for all projects where this is relevant.

In short, having this possibility in Feliz would make things a lot easier for maintainers of bindings where this is necessary.

I agree that it's low-level and that ideally, bindings returning something to be spread would return IReactProperty [], but it can cause notably higher maintenance.

Just sharing thoughts here; ideally I'd like F#-idiomatic bindings everywhere, of course.

[Zaid-Ajaj]

But what about MUI's mixin? It's defined some levels down (theme.mixins.toolbar) in interfaces that are only used for directly accessing native JS code, as is commonly seen in bindings. Changing this to return something else would mean that everything up the hierarchy would also need to be modified to not be interfaces but instead classes with custom logic. It seems like this can, in general, cause a potentially big maintenance step up from (slightly tweaked) ts2fable bindings, just to change this one thing. And this would be necessary for all projects where this is relevant.

Can you show me an example?

In short, having this possibility in Feliz would make things a lot easier for maintainers of bindings where this is necessary.

I still think these primitives shouldn't be exposed through Feliz, nor the binding that depends on it.

I agree that it's low-level and that ideally, bindings returning something to be spread would return IReactProperty [], but it can cause notably higher maintenance.

IMO, correctness and API ergonomics > maintenance because if you build an adhoc binding, the maintaince costs are transferred to the application code instead of the library code (which is what the library is trying to achieve: hiding the weird javascripty parts from the application code)

[cmeeren]

Hm, let's see.

Say I have the following binding type for some JS-native object:

type Mixins =
  abstract toolbar: obj with get, set

type Theme =
  abstract mixins: Mixins with get, set
  // lots of other members

I'd like to use theme.mixins.toolbar as IStyleAttribute []. Since toolbar: obj must exist in the bindings in one form or another (since that's how the JS API is), this leaves me two options:

1. Use them as abstract classes instead of interfaces, and add members with other names

I think I like this best, though it's certainly not perfect.

[<AbstractClass>]
type Mixins =
  abstract toolbar: obj with get, set
  member this.toolbarStyles =
    this.toolbar |> objectEntries |> unbox<IStyleAttribute []>

• Pros: Simple change, low maintenance, no quasi-duplicated types (which can also be confusing for users - for example, the above object may be created directly by the user, for example using jsOptions, when creating a custom theme)
• Cons: Bigger size since it now compiles as a class, the native API is always there in addition to whatever else you add, and you therefore have to find your own names if you can't add overloads. Also unsure if jsOptions etc. will create a POJO. Fable REPL indicates it will, but that may be an implementation detail for all I know. Edit: See next comment.

2. Have customized bindings in addition to "native" bindings

This is what I indicated in my previous comment. In addition to Mixins and Theme, we define:

type FelizMixins =
  abstract toolbar: IStyleAttribute list

type FelizTheme =
  abstract mixins: FelizMixins with get, set
  // lots of other members

Then we use this instead.

• Pros: We can stick to interfaces
• Cons: Every function in the JS API that accepts or returns a normal Theme must be wrapped in order to convert between FelizTheme and Theme. For object graphs running several level deep, this conversion may not be particularly fun to maintain, nor very efficient (everything is converted all the time, as opposed to option 1, where the conversion happens when you use a particular member).

Please ask if I'm not being clear enough.

[cmeeren]

Oh wait, it seems I can use extension members for option 1, meaning that some of the cons no longer apply: Fable REPL

[Zaid-Ajaj]

@cmeeren no extension methods needed, try this for option 1

[<AbstractClass; Erase>]
type Mixins =
  [<Emit("Object.entries($0.toolbar)")>]
  member this.toolbar : IStyleAttribute [] = jsNative

EDIT: this is of course only if you want to read the properties
SEE Fable REPL

[cmeeren]

Thanks! That's even better. I'll try to use that when I can. Do you have to use jsNative? I.e., no F# code for those members? I get errors if I try (which I guess makes sense, since it's erased?)

[Zaid-Ajaj]

When you use [<Emit(...)>], the expression on the left hand side is ignored altogether (actually if jsNative was to run, it throws an exception "js only" or something like that)

[cmeeren]

Sorry I was unclear; I was wondering if I had to use [<Emit(...)>] or if it was possible to use your suggestion also with F# code for the member. But that doesn't seem to work (ReferenceError: Foo$$get_B is not defined).

[Zaid-Ajaj]

Works if the member is inline

[<AbstractClass; Erase>]
type Foo =
  abstract A: int with get, set
  member inline this.B = this?A + 5

[Zaid-Ajaj]

Thanks! happy to help 😄

[cmeeren]

...Though that has the same considerations as other inline stuff, right? That if the body is not very short, it might be better to make it non-inline? I.e. use the Emit trick or extension members?

[Zaid-Ajaj]

Can we close this issue @cmeeren?

[cmeeren]

Sure!