[WIP] ReactComponent attribute

[alfonsogarciacaro]

See https://github.com/fable-compiler/Fable/pull/2066

This PR adds a ReactComponent attribute that acts as a Fable plugin and can generate some code (with a simplified version of Fable AST) during compile time to overcome the difficulties we've been having to define React components in pure F# code. Basically this attribute transforms a function like this:

module MyComponent

[<ReactComponent>]
let view model dispatch =
    // render code

Into something like:

const view = (function () {
   function MyComponent_view($props) {
      const model = $props.model;
      const dispatch = $props.dispatch;
      // render code
   }
   return function(model, dispatch) {
       return react.createElement({ model, dispatch });
   }
}());

So we get React-style props and proper naming for free, besides avoiding issue with generic props. The attribute works on module functions and static class member (so we can used named and optional arguments). Also this kind of attribute/plugins can easily be written by other libraries too, like Feliz. Is it cool or is not cool? ;)

@MangelMaxime After making some tests, maybe we can release an alpha/beta version of Fable.React v7 to replace the short lived v6. Although I guess we'll need a blog post and a way to warn the users they need to update their fable-compiler version.

[Shmew]

That is very cool! How would this work with things that have specific required types or parameters? Things like forwardRef come to mind, where it specifically needs a tupled argument with the second item being a React ref.

[Zaid-Ajaj]

Looks awesome :heart: I personally don't mind the current way of doing things but this makes it even eaiser!

What I really would like to have for Feliz is a plugin that re-writes the prop.children into static arguments rather than a runtime array, i.e.

Html.div [
  prop.style [ style.display.none ]
  prop.children [
     Html.h1 "first"
     Html.h2 "second"
  ] 
]

to be compiled as

createElement('div', 
  { style: .... }, 
  createElement('h1', null, "first"), 
  createElement('h2', null, second)
)

Rather than

createElement('div', 
  { style: .... 
    children: React.Children.toArray([
       createElement('h1', null, "first"), 
       createElement('h2', null, "second")
    ])
  })

To avoid the little extra runtime overhead imposed by Feliz. @alfonsogarciacaro do you think something like that is possible with the SimpleAst?

[alfonsogarciacaro]

@Shmew It's good you mention that :) At first I wanted to make this SimpleAst mostly untyped for simplicity, but I guess we'll have to expose the types somehow. Though we need to simplify the type system used in Fable too as, among other things, it keeps references to FSharpEntities and Fable.SimpleAst shouldn't have a dependency on FSharp.Compiler.Services.

@Zaid-Ajaj Right now it's not possible but we should likely provide a way to transform calls (an evolved Emit attribute) as well as declarations. The other tricky part is I'd rather not expose the Fable AST directly (so we don't break the plugins if we change it) which means the plugin cannot inspect Fable expressions only wrap them, but I'm thinking we could do as follows:

• Calling prop.children could tag the expression for later identification
• We could enable inspecting and transforming certain expressions like value or list literals. So when calling Html.div and the like the plugin could inspect the list, identify the argument with the children tag and take it out from the list to process it differently.

But, just out of curiosity, why do you need to call React.Children.toArray? Can't you pass the array directly to the children property? The overhead is calling React.Children.toArray or passing the children through a prop instead of as spread arguments?

[Zaid-Ajaj]

But, just out of curiosity, why do you need to call React.Children.toArray?

Because by default, React will consider the children provided via the props dynamic when they come in as an array and React will start throwing warnings into browser console about missing keys in these elements even though when it is not needed. Calling React.Children.toArray adds keys to these elements automatically. It is a bit stupid that this is needed which is why I want to compile it away into spread arguments

The overhead is calling React.Children.toArray or passing the children through a prop instead of as spread arguments?

calling React.Children.toArray

[Shmew]

Would it be possible to in some way expose an interface for the Fable AST, so that if it's changed on the compiler side it could essentially be band-aid fixed until the next major release? I have no idea if that's possible/wise/or way too much work.

[Zaid-Ajaj]

@alfonsogarciacaro I was thinking about this attribute today. Wouldn't it be much nicer if this was a plugin that can be configured such that it applies the transformation on all functions returning ReactElement?

[MangelMaxime]

@alfonsogarciacaro If it make better interop with react I think this is a nice addition.

After making some tests, maybe we can release an alpha/beta version of Fable.React v7 to replace the short lived v6. Although I guess we'll need a blog post and a way to warn the users they need to update their fable-compiler version.

I think we kind of need to wait for a beta of Fable v3 before releasing a new version of this library with plugin support. But indeed, we will definitly need a blog post explaining how it is used.

I was thinking about this attribute today. Wouldn't it be much nicer if this was a plugin that can be configured such that it applies the transformation on all functions returning ReactElement?

@Zaid-Ajaj

By making a plugin impact such a massive amount of code, we need to be sure to make some tests and be careful.

For example, would this work with Feliz code? <-- I suppose it would because it should only rewrap the user code.

Would it break some of the Fable.React Feliz helpers/specialized components?

For example, I don't think we want the plugin to impact ofInt, ofString, ofOption, mountById, etc. The functions are inlined so perhaps they would escape the patch.

What happens if there are two plugins trying to patch the same code? Which leads to is plugin order important?

[alfonsogarciacaro]

@Shmew Not sure what you have exactly in mind, but I'd rather don't touch the Fable AST directly. It would be another thing to worry about and it was already possible in Fable 1 but I didn't see any good case that benefited from the feature.

@Zaid-Ajaj That could be interesting, but in my experience Fable React applications use a lot of function helpers and it may not make sense to turn all of them into components. Besides, the attribute is also a way to load the plugin, if we don't have it we would need to find another way to attach the plugins to the compiler.

[Zaid-Ajaj]

After a long debugging session with the man himself @gaearon on how to make React/Feliz application compatible with fast-refresh we figured out what Fable needs to output when it comes when it comes to generating code to build React components. It boiled down to this list of requirements.

These requirements (PascalCase function, always exported without wrapping) can be achieved by slightly modifying the plugin attribute that implemented here in this PR which can be used both for Fable.React and Feliz.

Do you know whether t is possible to write a more specialized version of this for Feliz internals to make a definition like this

let counter = React.functionComponent(fun (count: int) -> ....

Translate to

const Counter = ({ count }) => {
   ....
}

Where the plugin/Fable extension is added in Feliz itself at React.functionComponent rather than having the users provide it?

@alfonsogarciacaro @MangelMaxime Can you please review the discussion as well? It is really important to understand how to make React hooks and state variables work out of the box with watch mode without anything else like a dedicated Elmish hmr package that we are currently using and it will streamline the experience even more.

@alfonsogarciacaro We had a really nasty issue where Fable introduces a function that wraps function components at call site of createElement, it would be great if we had some kind of attribute in Fable that customizes the call site of an external JS library without having to (I guess, uncurry) the used F# function which is in the case a React function component.

[alfonsogarciacaro]

This is great, @Zaid-Ajaj, thanks a lot for your work and thanks to @gaearon too for all the help. Plugins will likely make it into Nagareyama (Fable 3 codename) at the end. I think something as you ask should be feasible. Although I always prefer if the casing is set by users instead of having it modified by the compiler to avoid issues, but we could raise a warning if PascalCase is not used.

I'm having several ideas now for Nagareyama and I'm performing several thought experiments now :) But I'll try to publish an alpha version soon so we can start experimenting and see what works and what not. Please give me a few days.

About the wrapping. Yeah, this has been always tricky. For the F# compiler, a call to a method and a call to a lambda are different (because they're different in .NET world) so methods have to be converted to lambdas when passed as arguments. This looks as unnecessary wrapping in JS, but when I tried to remove it caused problems in other places. We do have an optimization to remove the wrapping when possible but it doesn't work all the times. Anyways, if we use a value instead of a function (as in let Counter = React.functionComponent the wrapping shouldn't happen.

[Zaid-Ajaj]

Plugins will likely make it into Nagareyama (Fable 3 codename) at the end. I'm having several ideas now for Nagareyama and I'm performing several thought experiments now :) But I'll try to publish an alpha version soon so we can start experimenting and see what works and what not. Please give me a few days.

That would be super awesome :heart: I would love to start playing with the plugin attributes. What I am most interested in is not only attributes that are used by library users but attributes for the library itself! Maybe if I could apply the attribute at the React.functionComponent level so that it could automatically rewrites its own body expression 🤯 and the only change required by users is to make the let bindings pascal case.

I am also thinking of cases where I could write specific attributes for specific libraries to gain even more control over the generated code in the expression body and users wouldn't really notice. A major use-case for me in Feliz is to re-write prop.children into spread arguments and to beta-reduce style properties into an object expression. These things currently happen at run-time and it is quite ugly to read.

Anyways, if we use a value instead of a function (as in let Counter = React.functionComponent the wrapping shouldn't happen.

I am afraid we tried this and the wrapping occurs anyways. I believe because functionComponent returns a function, Fable still deals with Counter as if it was a function at call site

[alfonsogarciacaro]

There will be different kind of plugins. Right now I'm thinking of the following:

• DeclarationPlugin: Modifies a declaration, as in this PR.
• CallPlugin: Intercepts a call to a method and emits custom AST. I think this should fit your purposes.

• ReplacementPlugin: Intercepts a call to another method (from a BCL class for example) and emits custom AST or redirects the call.

  I am afraid we tried this and the wrapping occurs anyways. I believe because functionComponent returns a function, Fable still deals with Counter as if it was a function at call site

Ah, ok. Hmm, we'll have to look into that then. Some possible solutions from the top of my head are: make functionComponent return a function instead (but that would be a breaking change), use CallPlugin in createElement to identify and remove the wrapping, try to implement the unwrapping better in Fable (maybe in the last step Fable2Babel).

[Zaid-Ajaj]

@alfonsogarciacaro I think the DeclarationPlugin and CallPlugin would be extremely useful (even for other scenarios) 😍 any chance we can get something to play with pre-alpha-ish?

[alfonsogarciacaro]

Superseded by #207