Zaid-Ajaj / Feliz

A fresh retake of the React API in Fable and a collection of high-quality components to build React applications in F#, optimized for happiness
https://zaid-ajaj.github.io/Feliz/
MIT License
544 stars 83 forks source link

Use units of measure instead of length.px etc.? #37

Closed cmeeren closed 5 years ago

cmeeren commented 5 years ago

So this probably won't lead anywhere, but I wrote the whole issue before I did some calculations and saw how it might fail, and other people may come up with clever solutions, so here goes anyway:

One (slight) annoyance I have with Feliz is constructing CSS lengths:

style.height (length.px 200)

The following is more readable, using F#'s units-of-measure syntax:

style.height 200<px>

To accomplish this, we need to define all units:

// Could be auto-opened
module CssUnits =
  [<Measure>] type px
  [<Measure>] type em

Then, all members that now accept ICssUnit must instead have one overload for each unit:

  type style =
    static member inline height(value: int<px>) =
      Interop.mkStyle "height" ((unbox<string>value) + "px")
    static member inline height(value: float<px>) =  // ...
    static member inline height(value: int<em>) = // ...
    static member inline height(value: float<em>) = // ...

Since there are 32 different length members (including auto, which I don't have a good solution for), each style members that now has a single overload that accepts a single ICssUnit instead gets 32 overloads each, one per unit.

It should be fairly easy to write a generator for this, where you just have a list of units and partial member definitions, and generate all members/overloads from this.

Unfortunately, one big problem is the combinatorial effects of multi-parameter functions, such as style.margin(top, right, bottom, left). If we want to have all possible combinations of units, this method alone gets 1,048,576 overloads just from the unit combinations. That's of course completely out of the question. I don't know if there is a solution to that, other than requiring every parameter to be the same unit, making it linear, not polynomial.

One could of course have an intermediary helper function convert from any unit to the existing ICssUnit, e.g. style.height (length.parse 200<px>), but then we're back to scratch with the only difference that the unit is after the number (good) and it's more verbose and less discoverable (bad).

Feel free to just close this if you want.

PS: A question that pops to mind is what the length type and the ICssUnit actually accomplishes. If length.px 200 just converts to the string "200px", why not just write "200px" in the first place? What additional safety does the length type provide? (I mean, sure, it stops you from passing invalid strings like "foo" or "200invalid", but I can't imagine that will ever be a problem in practice.)

MangelMaxime commented 5 years ago

The code you provided isn't valid F#, you will have this error:

Duplicate method. The method 'height' has the same name and signature as another method in type 'style' once tuples, functions, units of measure and/or provided types are erased.

Because unit type are erased at compile time you end up with something like:

  type style =
    static member inline height(value: int) =
    static member inline height(value: float) =  // ...
    static member inline height(value: int) = // ...
    static member inline height(value: float) = // ...

As you can see it result with a duplicate int and float methods.

In the past, I made some experimentation around providing helpers for working with units. https://github.com/Zaid-Ajaj/Feliz/issues/4#issuecomment-513723184

But all these solutions, for now, are going to impact the bundle size until the issue about F# feature is done.

cmeeren commented 5 years ago

Ah, my bad. So it won't work at all anyway.

cmeeren commented 5 years ago

Closing since it won't work.

Now this is slightly off-topic, but if anyone has any input on the question in the final paragraph, I'd appreciate to hear it! :)

MangelMaxime commented 5 years ago

Oops, I forget to answer the question, well the main benefit I see if you can directly do the calculation.

length.px 200+100

cmeeren commented 5 years ago

I agree and would, all else being equal, slightly prefer length.px (200+100) over string (200+100) + "px". However, the latter seems almost as simple and is just a few characters longer, so if that's the main benefit, is the whole length/ICssUnit stuff really justified?

Just putting it out there. :)

zanaptak commented 5 years ago

I also think there is room to explore regarding unit definition, and briefly experimented a bit with something like this, but didn't pursue further as length.xxx is "good enough":

// style.margin.px 20
// style.padding.rem 1.5

module Util =
  // SRTP to allow any numeric for units
  let inline rem< ^a when ^a : ( static member get_One : unit -> ^a ) > ( value : ^a ) = string value + "rem"
  let inline px< ^a when ^a : ( static member get_One : unit -> ^a ) > ( value : ^a ) = string value + "px"
  // and so on...

module margin =
  let inline rem v = Util.rem v
  let inline px v = Util.px v
  // and so on...

On the combination aspect, I don't think it's necessary to support all possible combinations since you can always break down combined properties if you need to mix units (e.g. use marginLeft/Right/Top/Bottom rather than margin w x y z).

Having the units as types rather than strings is so that something like "1.5ren" would fail at compile time, rather than runtime where it could be harder to debug.