EraseNamed unions?

[alfonsogarciacaro]

When I saw the bundle size increase when compiling fulma-demo with Nagareyam I tried to find a solution for it. Then I remembered when we tried to turn unions into arrays (as Bucklescript does) for Fable 2 (but we couldn't if we wanted to keep full compatibility with F# unions). I came up with the idea of a new attribute to create EraseNamed (tentative name) unions. This is explained below but the idea is to put the case name as the first element of the array.

In a quick prototype with a hack to turn all unions in Fulma and Fable.React.Props namespaces into EraseNamed unions the bundle size of fulma-demo went from 221K to 184K so closer to what we have now with Fable 2.

Erase unions

• Single case/single field (Fable 2): Erases to single value Foo 5 -> 5
• Single case/multiple fields (new in Nagareyama): Erases to tuple (JS array) Foo(5, "a") -> [5, "a"]
• Multiple cases/single fields (Fable 2): Erases to single value to emulate Typescript union types, union test converts to type testing.
• Multiple cases/none or multiple fields: Not allowed

EraseNamed (tentative name) unions

Same limitations as Erase unions: cannot implement interfaces or override ToString/Equals, no reflection info. They erase to a tuple where the first element is the case name: Foo(5, "a") -> ["Foo", 5, "a"]

The main advantage is this allows for pattern matching, so we can perform most of the things we do with normal unions. Also, because we keep the case name, these unions are compatible with keyValueList.

Pros

• The bundle size seems to decrease. This can have more effect in bigger apps. Maybe we can even get better results by erasing all the Msg unions in Elmish apps.

Cons

• We may be complicating things too much
• Easier to confuse Erase and EraseNamed unions. Both conceptually and also by making mistakes when adding the attribute.
• No matter how many times we warn about it, some users will still request/expect the erased unions to have the full power of "proper" unions.

Notes

• Still haven't figured out why erasing the unions makes for a smaller bundler, the code generated should be more or less the same (union constructors don't include the case name now). Removing the union class declarations definitely helps but not sure if this accounts for the whole difference. Maybe the minifier works better with tuples (JS array literals) because it can move them more agressively?
• At one point I broke the ParamList attribute and an empty array was emitted for the children of void elements (e.g. react.createElement("img", { src: "" }, []) which made React fail... but it made the bundle smaller! 184K -> 167KB 😮 Still puzzled by that.
• When gzipped, the difference is obviously smaller (maybe not worth the trouble?): 43K vs 39K

Thoughts? @ncave @Zaid-Ajaj @MangelMaxime

[ncave]

@alfonsogarciacaro Is that minified or unminified bundle size, do you mind posting the build steps for one of those projects so we can reproduce?

[alfonsogarciacaro]

An alternative would be to have a single attribute and use an argument in the only case of ambiguity (multiple cases with single field), we would throw a warning of there's no argument.

[alfonsogarciacaro]

It's minified bundle, the build step is to clone the nagareyama branch of my fulma-demo fork (see MangelMaxime/fulma-demo#43) and do:

yarn # npm install
dotnet tool restore
dotnet fable src --run webpack
ls -lsh output/*.js

To try a local version of fable I usually do the following (I will push the branch with the erased unions with name prototype later).

dotnet run -c Release -p ../fable2/src/Fable.Cli -- src --run webpack

[ncave]

@alfonsogarciacaro I like it, it's a more powerful version of the Erase attribute. Perhaps EraseUnion or ErasedUnion instead of EraseNamed?

[ncave]

@alfonsogarciacaro

we can perform most of the things we do with normal unions

Can this be the automatic default then for cases that can be fully handled, and fall back to the other Union definition if needed? What cases cannot be handled with a named array representation?

If we don't want to break compatibility, can we have it enabled as a compiler flag, say --erasedUnions?

While we're at it, is there any benefit of erasing records too to array representation, or is that going too far ;)

[Zaid-Ajaj]

Personally right now, I would love if libraries moved away from lists of unions when implementing React properties, they have lots of limitations which can be a huge PITA. If you ask me, I would say we should not be investing more time and energy in them but keep them working as is:

• Not being to able to overload the same property with different primitive values, having to resort to hacky U<_,_,_..., _> in the public API which make consumer code look really ugly
• Not being able to transform the input into a modified version of what the underlying React library expects. The best example here is when working with function properties where we e.g. in Feliz transform it into Func<, > internally without having end users be bothered with JS internals

The alternative is simple:

type IReactProperty = interface end 

[<Erase>]
type library = 
    static member inline content(value: int) : IReactProperty = unbox("content", value)
    static member inline content(value: string) : IReactProperty = unbox("content", value)
    static member inline content(value: float) : IReactProperty = unbox("content", value)

• No ambiguity on the the eventual property name would be (that is just the first item of the tupe)
• Easily overloadable
• Easily extendable without Custom(key, value)
• No extra bundle-size from outer API

[bklop]

I just compared the production bundle size (excluding vendor packages) for a large proprietary Elmish app where regular, non-erased unions are used extensively throughout the domain model.

Fable 2.13.0: 1268 KiB
Fable 3.0.0-nagareyama-beta-001: 1288 KiB

So the relative increase in bundle size isn't that dramatic and definitely something we can live with.

Note that we're using Webpack 5, which has better optimizations than Webpack 4 (including better tree shaking). We noticed a decrease in bundle size when upgrading from Webpack 4 to Webpack 5 (using Fable 2).

[alfonsogarciacaro]

This is super helpful @bklop, thanks a lot! Yes, I think I'm putting too much focus on fulma-demo. This is quite a small app so the apparent benefits you get from removing the unused Fable.React and Fulma literals dissipate as the main app grows in size and more features are used. I've also noticed that putting all case names in an array plays better with gzip, even with level 1 compression seems the excess KB get compressed at a very well ratio (around 90%). We shouldn't add complexity for the user for little gain.

After sleeping over the idea I'm less convinced of it. We've tried similar things in the past and there are always corner cases (equality, serialization...) that cause issues. I also agree with @Zaid-Ajaj that we should promote methods over unions for building the props of external React components.

Let's close this for now, we can revisit later if need 👍

[ncave]

@alfonsogarciacaro Would you still push your branch even if unofficial, it would be interesting to test the performance difference.

[alfonsogarciacaro]

@ncave Funny thing is I had some problems because the OS was locking files in my local repo and I cloned it again so the branch is lost 😅 But I've pushed #2236 with a similar idea, please have a look if you're interested.