Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Road to Fable 3 #2078

Closed
alfonsogarciacaro opened this issue Jun 9, 2020 · 14 comments
Closed

Road to Fable 3 #2078

alfonsogarciacaro opened this issue Jun 9, 2020 · 14 comments

Comments

@alfonsogarciacaro
Copy link
Member

So far there have not been many good news in 2020 so let's to make one by releasing a new major version of Fable 3 😄 I'm collecting different threads from recent issues here to list the features that could go in Fable 3:

Reflection support

My original hope was that #1839 would basically become Fable 3, but I've left too much time passed, the PR has lost sync with master and it's proving a bit difficult to make it work now. Besides I had some concerns about its reflection support. Although it's very interesting to be able to access to all class methods, constructors, properties and attributes (currently Fable is limited to record, union and tuple fields) for scenarios like test generators (I'm also using reflection now for interacting with databases in Node with good results), I'm concerned about how this would affect the resulting bundle in web apps.

Fable tries to play well with tree shaking in JS. For example class methods are not attached to the objects and can be removed from the bundle if not actually used. However, if we keep references to the methods in order to be able to call them through reflection, tree shaking won't be able to tell unused code apart (e.g., see this comment about Dart).

In some quick tests, the increase in the bundle size was not much, but I'm not sure if reflection wasn't used much in those cases. When you include support for quotations the bundle size explodes and it could be because reflection is used more intensively.

I'm considering now to give support to Constructors, Methods, Properties and Attributes, but only through special helpers in Fable.Core.Reflection module that generate the information in compile time only where it's needed (same way as we can only get type info when the type is known at compile time). Like:

open Fable.Core

let cons = Reflection.generateConstructors<MyType>()
let methods = Reflection.generateMethods<MyType>()
let meth = Reflection.generateMethod<MyType>("Foo")

// For example, this would be replaced in compile time by
// an array with the attributes decorating the type
let atts = Reflection.generateCustomAttributes<MyType>()

Would that be a good idea? The main downside is there could be duplicated code if we make the same call in multiple places.

Quotation support?

This also depends on #1839 Not sure how difficult would it be to add quotation support if we implement reflection differently.

Mangling abstract members

See #2068. Basically, right now we cannot implement two interfaces with the same member name in Fable. If we want to allow this we need to mangle the member names so they don't conflict when attached to the object prototype. But then interfaces used to type objects from JS would have to be decorate with an attribute like NoMangle or similar.

This would be the main breaking change in Fable 3, but we could have a transition period.

Type testing

The main limitations of type testing and Fable atm are:

For generics maybe we can try injecting the name of the generic arguments in the constructor of generic types (something similar was done in Fable 1) although we would still have issues with types that translate directly to JS counterparts like arrays or mutable dictionaries.

The main reason for not testing interfaces was they might represent objects from JS, but if we're going to restrict this to interfaces decorated with NoMangle in Fable 3, we could add the name of implemented interfaces to F# objects (again, something similar was done in Fable 1).

Plugins

See #2066 and fable-compiler/fable-react#196

Magic?

@ncave is up to something, we just don't know what yet 😉

@MangelMaxime
Copy link
Member

@alfonsogarciacaro Would this new addition to the reflection helps provide support for Classes, Interface to Thoth.Json? If not, what would still be missing?

In some quick tests, the increase in the bundle size was not much, but I'm not sure if reflection wasn't used much in those cases. When you include support for quotations the bundle size explodes and it could be because reflection is used more intensively.

IHMO an increase of 50% on a project which doesn't use reflection is kind of a big impact. I don't think that fable2-samples/browser and fulma-demo are using any sort of reflection API.

I just want to remember that one selling point of Fable 2 was the bundle size reduction because it was something that people kept talking about in Fable 1. Of course, nothing prevents us to decide that the bundle size is no more a top priority. Also, a bigger bundle would lead to slower page load and slower first-time execution I think because more code need to be parsed by the VM.

@krauthaufen
Copy link
Collaborator

I think it would be ideal to disable reflection as much as possible for optimizing bundle size but I think we shouldn't drop/disallow powerful features like quotations just because they result in larger outputs...

Maybe we should just keep the compiler flag for enabling/disabling quotation support...
As for the larger footprint we could certainly optimize the reflection code, but wouldn't it overall be better to include a dead-code elimination that doesn't rely on webpack but instead operates directly on fable's AST?
This optimization could (at least theoretically) be better than webpack's tree shaking approach since webpack can only operate per exported function and cannot drop things like interface members as a whole...

I'm aware that fable generates code that tries to be optimal for this scenario, but some concepts (like virtual calls) don't really work with this approach, which means they can never be removed, and therefore everything they call must also exist (btw. thats the main reason why quotations result in huge bundles, since alle MethodInfos provide invoke members)

@MangelMaxime
Copy link
Member

@krauthaufen I am totally OK for adding powerful features :) It is just that I don't think it should have a 50% bundle increase for all the users while most of them don't use the feature.

I do hope we can find a compromise :) either by optimizing the reflection code or using a flag as you said :)

@alfonsogarciacaro
Copy link
Member Author

@MangelMaxime Sorry, I just realized I linked the wrong comment about the bundle increase. After some improvements by @krauthaufen the bundle increase was 152kB vs 136kB which is not that bad.

@krauthaufen So you mean adding an extra pass in Fable that detects unused interface methods so we can remove the implementations? If we use now the NoMangle attribute to tell interfaces used to interoperate with JS apart, this could work and be an interesting addition to Webpack's tree shaking which already works quite well with static code. Although I'm not sure if we could use it for the reflection as this is part of the fable-library code which is either in Typescript or precompiled.

@MangelMaxime (again) About Thoth.Json, that would be difficult. It already is in .NET:

  • Interfaces don't have a constructor so we don't know how to instantiate the type. Actually we could hack it in Fable as there's no interface type checking just by validating the fields of the json object. But in .NET we would have to use System.Emit to generate code that creates the interface and I don't think we should do that.

  • Classes can have arbitrary constructors and arbitrary runtime representations. IIRC the way Json.Net supports classes is by calling an empty constructor and then filling the properties, but this is just because C# didn't have records. The are ways to customize the json serialization with attributes, etc. But in Thoth.Json there's already Extra.withCustom which I think does the job nicely.

@krauthaufen
Copy link
Collaborator

Hey, I was thnking about something along the lines of FShade's Optimizations but in a more sophisticated way, since FShade doesn't eliminate fields, etc. currently. All MethodInfo.Invoke implementations could for example be eliminated when no one ever calls any of them.

However you're right about the Fable-Library problem, so we would need to translate typeof<MyType> to a call that takes some sort of config like { needInvoke : bool; needFields : bool}

@MangelMaxime
Copy link
Member

@alfonsogarciacaro Thank you for the explanations. 👍

And indeed, this bundle increase seems better even if it seems to say that when using the "new reflection" the increased size is important.

I just wanted to mention that increase bundle size need to be monitored and I think it is easier to wait for beta and see what it will imply. :)

@uxsoft
Copy link

uxsoft commented Sep 2, 2020

Any plans for F# 5?

@ncave
Copy link
Collaborator

ncave commented Sep 2, 2020

@uxsoft When F# 5 is out of preview, we can technically release it for both Fable 2 and Fable 3.

@uxsoft
Copy link

uxsoft commented Sep 2, 2020

Does it mean that there is no work that has to be done to support for example string interpolation?

@ncave
Copy link
Collaborator

ncave commented Sep 2, 2020

@uxsoft There will be some work to reference the latest FCS (FSharp Compiler Service), but it's not dependent or attached to Fable 3 work, it can go in Fable 2 as well. Once F# 5 is out of preview (or technically even before that if it's feature-stable) we can proceed.

@Shmew
Copy link
Contributor

Shmew commented Sep 21, 2020

I imagine the change I'd like done would need to go into Fable 3 since it would be a pretty breaking change, but would significantly improve using JS native types. My suggestion is to remove the use of interfaces completely in Fable.Core.JS in favor of actual types. We can also add constructs and additional helpers that F# developers would expect such as actual constructors and module style methods like myTypedArray |> JS.TypedArray.map (...).

This could all be done in a way that there is no actual bundle overhead as well. It also makes the interface mangling thing a non-issue.

I'm also curious, is AllowNullLiteral still necessary on those types?

I don't mind at all doing this work, and I can slap the core change together real quick and put in a draft PR so you can see what I mean.

@Shmew
Copy link
Contributor

Shmew commented Sep 21, 2020

I just created a draft PR that has the conversion without any additional work done (like the modules and stuff).

@MangelMaxime
Copy link
Member

I propose to move the discussion about @Shmew proposition in his PR. (I already started asking questions there ;))

@alfonsogarciacaro
Copy link
Member Author

Closing this as discussion has moved to different places and all the information here may not be up-to-date, please check the Nagareyama project to keep track of progress for the next major release: https://github.com/fable-compiler/Fable/projects/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants