3rd party guide

[cmeeren]

This looks fantastic IMHO.

Is this simple to extend for 3rd party stuff, such as Material-UI?

It would be great to have some kind of guide on how to do that. :)

[Zaid-Ajaj]

This looks fantastic IMHO.

Thank you! Glad you like it 😄

Is this simple to extend for 3rd party stuff, such as Material-UI?

Sure thing, but the approach is bit different because we are not using discriminated unions. I try to write something up when time permits but the idea is that you essentially have two options:

• 1. Extend the prop static type with more functions
• 2. Create a similar prop static type, specific to your library (maybe Mui) which has all the properties that the Mui requires. You could even alias prop with Mui and extend Mui with Mui-specific (overloaded) properties and functions.

Extending is as simple as:

type prop with
  static member hello (value: string) = Interop.mkAttr "hello" value 

Whether this is the "final form" of the library and the way to extend it, is still under consideration as I first want to try it out by building 3rd party libraries and see how it goes

[cmeeren]

Thanks! I'm already well underway with testing something for MUI, just for a fraction of the API, to see how it works. Will probably share sometime during the week/-end, it would be great if you could have a look at it just to see if I'm on track and following the Feliz spirit. Seems like success so far!

However, I haven't extended the existing prop type. I have defined a separate prop type in another namespace (Feliz.MaterialUI). Works seemingly great; you of course get access to all members from all matching types if you open both Feliz and Feliz.MaterialUI.

I have a type Mui that corresponds to Html and contains the actual components.

(Currently I have placed component-specific props in separate submodules of prop, as mentioned in #13.)

A possible point of improvement in Feliz is to have a reactElement and createElement that accepts not string, but ReactElementType (I think). so that we can call createElement (importDefault "@material-ui/core/Button"). I have created these two helpers myself currently.

By the way, should all member be inline? What are the pros/cons? I notice you didn't use inline above, but everything in Feliz is inline.

[Zaid-Ajaj]

Thanks! I'm already well underway with testing something for MUI, just for a fraction of the API, to see how it works. Will probably share sometime during the week/-end, it would be great if you could have a look at it just to see if I'm on track and following the Feliz spirit. Seems like success so far!

That's awesome! I would definitely be happy to take a look if you

However, I haven't extended the existing prop type. I have defined a separate prop type in another namespace (Feliz.MaterialUI). Works seemingly great; you of course get access to all members from all matching types if you open both Feliz and Feliz.MaterialUI.

I have a type Mui that corresponds to Html and contains the actual components.

That's what I would do for Mui

(Currently I have placed component-specific props in separate submodules of prop, as mentioned in #13.)

It makes sense for Mui because of how many options you have

A possible point of improvement in Feliz is to have a reactElement and createElement that accepts not string, but ReactElementType (I think). so that we can call createElement (importDefault "@material-ui/core/Button"). I have created these two helpers myself currently.

I am reviewing the members I am using in the Interop module, I just exposed whatever I was using in the library, will be reconsidered for the stable release

By the way, should all member be inline? What are the pros/cons? I notice you didn't use inline above, but everything in Feliz is inline.

I should have inlined the extended member above!

The rule of thumb is: if the value of the property is primitive like a string/int/bool/enum then inline the property but if your property computes a value based on the input then it is better not to inline because every time the user calls the inlined function, the whole function body is inlined in that place of invocation, so if a user uses the same function 10 times throughout the application, the function body is compiled inline 10 times instead of being defined once and referenced 10 times

[cmeeren]

The rule of thumb is: if the value of the property is primitive like a string/int/bool/enum then inline the property but if your property computes a value based on the input then it is better not to inline because every time the user calls the inlined function, the whole function body is inlined in that place of invocation, so if a user uses the same function 10 times throughout the application, the function body is compiled inline 10 times instead of being defined once and referenced 10 times

Nice to know! But (in the context of Fable) why inline in the first place, then? What's the benefit for the "simple" function/method bodies?

[Zaid-Ajaj]

• Bundle size: if these functions aren't used (and there are 100s of them), they are still compiled anyway, inflating the bundle size AFAIK (the impact is of course still relative to total size of the application)
• Generic type arguments: irrelevant for Feliz but in context of Fable, a generic function with generic type arguments will not compile if the place of invocation is not inlined (all the generic type args have to be statically resolved at compile time) except for special cases where you can use [<Inject>] ITypeResolver<'t> as an optional argument of a static class (only highly specialized libraries use this feature, see Fable.SimpleJson/Thoth.Json)

[Luiz-Monad]

I think babel does tree-shaking and removes unused functions when you make the production bundle. Inlining would defeat that.

[cmeeren]

@Luiz-Monad Are you then saying that, ideally, nothing in Feliz should be inlined? That inlining for bundle-size reasons is counter-productive?

[Zaid-Ajaj]

@Luiz-Monad What you are saying would be awesome! At least if compilation worked that way. Here is an example you can try with the REPL:

module App

type prop = 
  // does useless stuff
  static member f() = 
    [ 1 .. 100 ]
    |> List.map (fun x -> x * 20)
    |> List.collect (fun n -> [n; n])
    |> List.fold (+) 0

  // does useless stuff
  static member inline k() = 
    [ 1 .. 100 ]
    |> List.map (fun x -> x * 20)
    |> List.collect (fun n -> [n; n])
    |> List.fold (+) 0

  static member g() = 1

let value = prop.g()

printfn "%d" value

Where prop contains:

• f() contains a function body -> not inlined and also not used
• k() contains a function body -> inlined but not used
• g() contains a function -> not inline and used

You would think that both f() and g() won't be compiled but that is not the case, f() (not inlined and not used) is compiled anyway, but k() (inlined, not used) doesn't make it into the compiled bundle

import { fold, collect, map, ofSeq, ofArray } from "fable-library/List.js";
import { type } from "fable-library/Reflection.js";
import { rangeNumber } from "fable-library/Seq.js";
import { toConsole, printf } from "fable-library/String.js";
import { declare } from "fable-library/Types.js";
export const prop = declare(function App_prop() {});
export function prop$reflection() {
  return type("App.prop");
}
export function prop$$$f() {
  return fold(function folder(x$$1, y) {
    return x$$1 + y;
  }, 0, collect(function mapping$$1(n) {
    return ofArray([n, n]);
  }, map(function mapping(x) {
    return x * 20;
  }, ofSeq(rangeNumber(1, 1, 100)))));
}
export function prop$$$g() {
  return 1;
}
export const value = prop$$$g();
toConsole(printf("%d"))(value);

[Luiz-Monad]

It does indeed work, but you need to configure webpack for doing that, because what does the tree-shaking is not fable itself, so it won't work in the REPL.

Before

/// Library.fs
module Library

type prop = 
    // does useless stuff
    static member f() = 
      [ 1 .. 100 ]
      |> List.map (fun x -> x * 20)
      |> List.collect (fun n -> [n; n])
      |> List.fold (+) 0

    // does useless stuff
    static member inline k() = 
      [ 1 .. 100 ]
      |> List.map (fun x -> x * 20)
      |> List.collect (fun n -> [n; n])
      |> List.fold (+) 0


type AppMain =
    static member g() = 1

//// App.fs
module App

let value = Library.AppMain.g ()

printfn "%d" value

After

  declare(function Library_prop() { 
     // see its empty, this weren't removed too because of `keep_classnames: true, keep_fnames: true ` in the terser plugin
  });
  declare(function Library_AppMain() {
  });
  !function toConsole(arg) {
    return arg.cont(function (x) {
      console.log(x)
    })
  }(printf('%d')) (1),
  __webpack_require__.d(__webpack_exports__, 'value', function () {
    return 1
  })

Also, there's a caveat to your test, the entry file is kind of special in that functions from it aren't removed. ( I guess there's something to do with initialization of static classes, something has to call static initializer constructor to do side-effects on the modules )

See this repository I made for this testing https://github.com/Luiz-Monad/test-tree-shaking

[Zaid-Ajaj]

Thanks a lot for the example repo! I will definitely investigate this further to see if inlining is actually doing anything useful in the context of Feliz

[cmeeren]

I will definitely investigate this further to see if inlining is actually doing anything useful in the context of Feliz

Cool, looking forward to hearing what you find :)

[cmeeren]

That's awesome! I would definitely be happy to take a look if you

Please check out the feliz branch on cmeeren/fable-elmish-electron-material-ui-demo.

Most of the code is auto-generated based on the (HTML) API docs. There's a generator project (ugly and hacky, but gets the job done) and a project for the actual bindings. In the renderer project, only App.fs has been converted to use the new Feliz-style bindings.

Please have a look at it when you feel like it, and let me know what you think and if you have any questions.

[Zaid-Ajaj]

@cmeeren Looks very nice and much better than the current API IMHO but is it still a bit hard to read but that is more the nature of the library itself, it has a lot of very specific parts that you have to familiar with. I think there are parts that could be improved, take this example:

Mui.appBar [
  prop.className c.appBar
  prop.appBar.position.fixed'
  prop.children [
    Mui.toolbar [
      prop.children [
        Mui.typography [
          prop.typography.variant.h6
          prop.typography.color.inherit'
          prop.text (pageTitle model.Page)
        ]
      ]
    ]
  ]
]

My personal perfect version of this snippet would be to turn it into the following:

Mui.appBar [
  AppBar.className c.appBar
  AppBar.position.fixed'
  AppBar.children [
    Mui.toolbar [
      Toolbar.children [
        Mui.typography [
          Typography.variant.h6
          Typography.color.inherit'
          Typygraphy.text (pageTitle model.Page)
        ]
      ]
    ]
  ]
]

This way it is easy to find Mui elements because you can just "dot through" Mui and once you found your element (appBar), you can "dot through" the module name (AppBar) to define the properties and such

Maybe keep AppBar as lowercase as well

I think you get the idea but to be more exact, the general syntax of this API is as follows where {Element} is a react element:

Mui.{element} [
  {Element}.{propName}.{propValue}
  {Element}.children [
    Mui.{otherElem} [
      {OtherElem}.{propName}.{propValue}
      // etc.
    ]
  ]
]

What do you think about this? This API also inspired the library the ideas of fabulous-simple-elements if you want to see a concrete implementation

[cmeeren]

I think that's perfect, and just the kind of thing I wanted to get your opinion on. I initially chose to have everything under prop since that's how the main library worked, but of course you don't really have any component-specific props, whereas MUI has more more less only component-specific props.

I think sticking to lowercase module names may look best (and save an extra keystroke), but I'm open to counter-arguments.

Good that I have auto-generated this stuff, that makes it simple to change.

I'll update and let you know.

There's one thing in particular I'd like to get opinions on, though. It's the ClassName stuff. The makeStyles hook returns an object with the same props as the one it's passed in, but where every (JSS) element has been replaced by a string, which is the class name to use (and can be passed to e.g. prop.className).

Now, there's no way to type that in F#, so I have to work with what I have. Normally all props of the style object are IStyleAttribute list. This means that I can add an overload for prop.className which accepts IStyleAttribute list, which of course is a lie since at runtime it's a string. If you actually passed it IStyleAttribute list it would fail. In addition to prop.className, this also goes for all the classes.<element>.<classesName> (used in <element>.classes [ ... ]). They also accept a class name (string).

What I have done, as you can see, is to "require" all the IStyleAttribute list properties of the style object to be wrapped in asClassName, which basically just unboxes to IClassName (a proxy for string, if you like). And then I have added an overload to prop.className accepting IClassName, and made all the classes props accept IClassName. I like that it's more strongly typed, but I don't like that it requires extra typing (asClassName for every top-level CSS rule). The compiler will complain if you miss it, but it won't tell you what to do, and it's still extra noise.

Do you have any input on this?

[cmeeren]

Also, I noticed this:

listItem.divider ((page = Home))

Here, double parenetheses are required, since otherwise F# interprets it as trying to call listItem.divider with the (non-existent) page parameter set to Home (instead of the value parameter set to page = Home). Do you see a way to avoid that?

[Zaid-Ajaj]

Hello @cmeeren, first of all, I freaking love this syntax:

Mui.appBar [
  prop.className c.appBar
  appBar.position.fixed'
  appBar.children [
    Mui.toolbar [
      toolbar.children [
        Mui.typography [
          typography.variant.h6
          typography.color.inherit'
          prop.text (pageTitle model.Page)
        ]
      ]
    ]
  ]
]

It just looks so clean and so simple! Though if I were you, I would have probably duplicated some of the generic prop functions into component-specific props such as appBar.className instead of (or along side) prop.className so that they all look symmetrical but more importantly to give the IClassName overload to the Mui-specific component instead of the generic prop.className that takes a string because makeStyles is also a Mui-specific construct and it makes sense that it will only apply to Mui components.

I think you tackled the makeStyles the best way possible, at least right now I can't think of a better way (Although I am not big fan of asClassName, maybe Styles.createClass instead? it's up to you).

As for listItem.divider ((page = Home)) it is tricky, you could add a dummy function let when (x: bool) = x but that is just noise. I think it is best to file it as a compiler bug because I can't think of reason why F# compiler can't resolve the proper overload of the function, I haven't tried myself but I will look into it when time permits

Lastly, I am not as responsive as usual this week cause I am on vacation now, so I might not be able to read/check everything etc.

[cmeeren]

Though if I were you, I would have probably duplicated some of the generic prop functions into component-specific props such as appBar.className instead of (or along side) prop.className so that they all look symmetrical but more importantly to give the IClassName overload to the Mui-specific component instead of the generic prop.className that takes a string because makeStyles is also a Mui-specific construct and it makes sense that it will only apply to Mui components.

Check it out now :) I have made drastic improvements and expansions in the last days, just pushed them (not done, I'm currently going through all MUI components to verify and improve the generated props).

I think you tackled the makeStyles the best way possible, at least right now I can't think of a better way (Although I am not big fan of asClassName, maybe Styles.createClass instead? it's up to you).

I'd like to keep it as short as possible since it'll be used a lot, but I'm open for other names. Though I have half a mind to just have an IStyleAttribute list overload. It'll remove potentially quite a bit of noise, and I doubt it'll be very dangerous even if it can technically be used incorrectly.

As for listItem.divider ((page = Home)) it is tricky, you could add a dummy function let when (x: bool) = x but that is just noise. I think it is best to file it as a compiler bug because I can't think of reason why F# compiler can't resolve the proper overload of the function, I haven't tried myself but I will look into it when time permits

Thanks, I've filed an issue now: dotnet/fsharp#7423

Lastly, I am not as responsive as usual this week cause I am on vacation now, so I might not be able to read/check everything etc.

Gotcha, no problem. I'll keep posting issues if I come across things, and you just reply in your own time.

[Zaid-Ajaj]

Check it out now :) I have made drastic improvements and expansions in the last days, just pushed them (not done, I'm currently going through all MUI components to verify and improve the generated props).

Looks really good, with the generated docs too 😍 maybe time to put in it's own repo?

I'd like to keep it as short as possible since it'll be used a lot, but I'm open for other names. Though I have half a mind to just have an IStyleAttribute list overload. It'll remove potentially quite a bit of noise, and I doubt it'll be very dangerous even if it can technically be used incorrectly.

That would work too, Fable libraries cheat the type-system all the time ;)

Thanks, I've filed an issue now: dotnet/fsharp#7423

Awesome! thanks a lot

[cmeeren]

Looks really good, with the generated docs too 😍 maybe time to put in it's own repo?

I've been thinking about that, but there are still a few important bugs (e.g. #27) I'd rather figure out with the convenience of having everything in the same place, so I think I'll keep it there until it's ready for a prerelease on nuget (hopefully won't be too long).

[cmeeren]

@Zaid-Ajaj I'm just about done with Feliz.MaterialUI. Will publish to a separate repo soon. It would be great to have you review it, first and foremost to check some design decisions and ensure consistency with Feliz, but also to check some implementation stuff (e.g., am I using things you'll be making internal, or are there things I'm not using from Feliz but should be using).

When I create the new repo, is it OK if I create issues explaining what I'd like reviewed, and tag you?

[cmeeren]

I have now uploaded a draft of Feliz.MaterialUI to cmeeren/Feliz.MaterialUI. I have created several issues with things I'd like reviewed.

I would be very grateful if you could find the time to have a look at them!

There is no need to spend a lot of time on each issue; I really just want a second opinion for the reasons mentioned in my previous comment. I'm happy to go into the weeds if you want, but even just "this looks fine" would be great.

There's no rush, of course. :)

[Zaid-Ajaj]

Awesome work @cmeeren! At first glance, the bindings look very clean, I will take a look at each issue in the next few days, I promise 😄

[cmeeren]

Hey! Any chance to continue looking at the issues? Again no hurry, just a friendly reminder since I haven't heard from you for a couple of weeks 😃

[Zaid-Ajaj]

I have actually looked at the issues but like I said before, whether an API is good or not comes from use cases, that is why I asked you to publish an alpha version so that I could try it out but I haven't had the time do so yet (it has been on the back of my mind this week 😄)

[Zaid-Ajaj]

Hello @cmeeren, I guess we can consider this resolved, right? I will close the issue, please re-open for further discussion if needed