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

Helping with Fable 2 development #1370

Closed
alfonsogarciacaro opened this issue Mar 21, 2018 · 37 comments
Closed

Helping with Fable 2 development #1370

alfonsogarciacaro opened this issue Mar 21, 2018 · 37 comments

Comments

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Mar 21, 2018

Not sure if the dev2.0 branch is already in a state where I can start calling for help, but I'll do it anyway 😉 I'm listing below the main changes that are happening in that branch, how can you quickly test it out and which tasks are pending for the first alpha release. If you're interested in helping with any of them, please comment below so I can give more details to work together 💪

Non-comprehensive list of main changes

  • Fable AST now includes some of the most common constructs in F# (list, option...) so it's easier to change their representation in JS if necessary. Also, the AST now keeps type information from the F# compiler.
  • Records and unions won't be compiled as JS classes but as plain objects and arrays respectively.
  • All type methods are compiled now as JS module functions for better compatibility with tree shaking (hopefully it should also help JS engines optimize the code). The only exception is override methods (ToString, Equals), which are attached to the prototype. Also, nested module functions are compiled as functions of the file root module with mangled names.
  • When casting an instance to an interface, it will be wrapped with an object containing the interface functions, this prevents name conflicts among interfaces (and it's also necessary because methods are not attached to the prototype anymore).
  • Some modules in fable-core (like List or Array) are now written in F#
  • Optimizations applied to the Fable AST are now isolated passes happening in FableTransforms. This should make the code more maintainable than Fable 1, where optimizations are usually applied in multiple places (specially the uncurrying optimization, which hopefully will work much better in Fable 2). Some optimizations, however, still have to be done in the last Fable2Babel pass: tail-calls and decision trees (pattern matching).

Give it a quick try

  • Clone the repository if you haven't already
  • git checkout dev2.0
  • cd src/tools
  • bash quicktest.sh --build-core

This will compile and execute the QuickTest.fs file in the same folder. Feel free to use it as a playground to test Fable 2 features (but please don't include it in your PRs). The second time, if src/js/fable-core files haven't changed you can just run bash quicktest.sh (if src/dotnet/Fable.Compiler files haven't changed you can also run bash quicktest.sh --no-build but with 2.1.300-preview1 building a project with no changes should be quite fast anyways).

On Windows you should be able to run the script using Git bash.

Pending tasks

  • There is a number of TODO!!! comments spread through the code, I should probably handle those myself.

  • As plugins won't be included in the first alpha release, tests use now Expecto API, which is much easier to share with JS test runners. I've converted already some files (ArithmeticTests, ArrayTests, ListTests) but most of them are still waiting for some love. I was using Regex to change the signature of the methods to testCase but probably it's a good idea to write a script for it. One of the main challenges is that, given tests are values in a list now, extra functions that appear in the middle of the file must be moved to the top, which may shadow other values in some cases.

  • Set and Map modules in fable-core have to be ported to F#. I partially did it for Set (Map can also be taken from FunScript repo where I originally stole it took inspiration from). We need to be sure they are correctly compiled to JS, the public function names are exposed correctly, check if we need to port methods from the previous .ts files (saved in src/tools/old) and if the tests are working correctly.

  • Because the AST has changed so much, most of the Replacements module needs to be rewritten. I already did around 1000 lines and there's another thousand to go. This is a bit of a tedious task, but if someone is crazy enough to take on it, please let me know. Maybe the fastest way is to have a screen sharing session to show you what needs to be done and then split the remaining code so we can finish faster.

Things that won't be included in the first alpha release: bigint, reflection, plugins.

I hope that's enough info for now, thanks a lot for your help in advance!

@MangelMaxime
Copy link
Member

@alfonsogarciacaro I think I will take the Set and Map modules in fable-core task. Can you please give me some guidance or point me to the partially ported Set so I can take inspiration from here :) ?

@zaaack
Copy link
Contributor

zaaack commented Mar 22, 2018

Just a reminder, there are many repos that we can stole inspire from,

  1. visualfsharp's official core library, it's map implementation looks really like FunScript's:
    https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/FSharp.Core/map.fs
  2. OCaml/BucleScript's, there are not many differences between F# and OCaml's core syntax, I think we can easily porting them to F#.
    https://github.com/BuckleScript/bucklescript/blob/master/jscomp/stdlib/map.ml

@alfonsogarciacaro alfonsogarciacaro changed the title Helping with Fable 2 Helping with Fable 2 development Mar 22, 2018
@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented Mar 22, 2018

Thanks, @zaaack. Yes, the FunScript file was originally taken from FSharp.Core. Not sure if it's out of date but I tend to prefer it because it's already working (Map.ts in Fable 1 was also originally a translation of it) and I think it already chopped off some of the things we don't need in JS. About Bucklescript, I'm not sure if the fact that modules are values in OCaml could be problematic, also Map in F# must comply with some interfaces, and what took me most time is to make types with custom comparison work properly as keys, which I guess it's also somewhat different in OCaml.

BTW, Bigint is indeed taken from FSharp.Core but I think we port in a later release.

@MangelMaxime
Copy link
Member

Thanks @zaaack I will keep that in mind when working on the port.

@EdouardM
Copy link

Hi Alfonso I would like to help on the expecto testing part

@alfonsogarciacaro
Copy link
Member Author

That's great @EdouardM, thank you! It would be great if you could write a script to make most of the conversion automatically. If you want we can have a short screen sharing session to show what needs to be done :)

@EdouardM
Copy link

EdouardM commented Mar 23, 2018 via email

@EdouardM
Copy link

EdouardM commented Mar 23, 2018 via email

@jgrund
Copy link
Member

jgrund commented Mar 26, 2018

Things that won't be included in the first alpha release: bigint, reflection, plugins.

Just curious, is there a roadmap / plan to include these things post Alpha?

@alfonsogarciacaro
Copy link
Member Author

@jgrund No specific roadmap, but bigint should be easy to add (just port the code from FSharp.Core) and reflection should also be ready for the stable release. About plugins, I'd like to collect information about how they're being used and then decide together how we should bring them or whether they're needed at all.

@stkb
Copy link

stkb commented Apr 2, 2018

Thanks for this! I did have to make a small change to quicktest.sh to get it working in Git-Bash:

pushd ../js/fable-core
dotnet restore
bash quickfsbuild.sh --no-build
popd

Changed line 32 to simply:
./quickfsbuild.sh --no-build
since the bash in there (which I don't think is needed anyway?) was causing WSL to try to start up instead, which I don't have set up.

@alfonsogarciacaro
Copy link
Member Author

Hi @EdouardM! This what we discussed at fsharpX about updating the tests:

  • Using Expecto API (will be actually translated to mocha for JS)
  • Use regex let ``(.*)``.*= and replacement testCase "$1" <| fun () -> to change the signature of the tests
  • Remove the attributes [<Test>], [<TestFixture>]
  • Add the file to Main.fsproj and allTests in tests/Main/Main.fs
  • Special care is needed for async and functions in the middle of the tests, we can talk about that later.

@zaaack
Copy link
Contributor

zaaack commented Apr 10, 2018

@alfonsogarciacaro I'm reading dev2.0 branch's source code, trying to be the "crazy one" to working on the rest of Replacement.fs since the other two are both taken 😄. I hit some problems when porting parse function from the old Replacement.fs, the parse function depends on returnType which exists in Fable.ApplyInfo in Fable 1.0, but I cannot find it in CallInfo, don't know how to handle this, could you give me some help?

Here is my commit:
https://github.com/fable-compiler/Fable/compare/dev2.0...zaaack:dev2.0?expand=1#diff-a34e57b30a53951ff9b99ce4076feaa4R1390

@alfonsogarciacaro
Copy link
Member Author

That'd be great @zaaack, thanks a lot. And yes, sorry, I change a bit the signature of the functions in replacements to make them look more like functions in other modules, where in the first three arguments usually are (context may also come after in the second place):

let myFableHelper (com: ICompiler) (range: SourceLocation option) (typ: Type) ... =

For that reason I moved the return type (and range) from CallInfo to the first arguments of the function. You can use something like let parse com r t info thisArg args = (yes, I usually use just t and I agree it's not very meaningful).

I'm also working in the Replacements module. I can tackle the collections (Seq, List, Array) while you complete the other functions, what do you think?

I added recently a Helper static type in order to use static members with optional parameters. Please have a look at the existing code to see how the InstanceCall, CoreCall... helpers are being used. Other notes:

  • It's important to pass the argument types right (specially if there're functions) for the uncurrying optimization that happens at a later stage. If arguments are not modified pass them directly from CallInfo. If they are, arg types must be recalculated.
  • ThisArg is optional. If present, it will passed in the first place. In Fable 2, type instance members are actually compiled as static functions and that's why this is passed as an argument. ArgTypes don't include ThisArg.
  • However, when InstanceCall kind is specified, the ThisArg expression will be compiled as an actual instance call: thisArg.member(arg1, arg2...).

This should be enough for now :) Please check with me if you have any question and again, thanks a lot for your help!

@zaaack
Copy link
Contributor

zaaack commented Apr 10, 2018

Thanks for your quick and detailed reply, I'll try it tomorrow.

I'm also working in the Replacements module. I can tackle the collections (Seq, List, Array) while you complete the other functions, what do you think?

Great, I think I can spend two hours on weekdays and full day on weekends, mostly.

@MangelMaxime
Copy link
Member

Just for info, if someone is porting the test files no need to port SetTests.fs and MapTests.fs I will do it to test the module intergration.

Also, if you use VSCode, you can use multiple selection to do all the work at once inside a file. Writing a scripting seems a bit over engeneering here :).

2018-04-18 15 14 27

@EdouardM
Copy link

EdouardM commented Apr 18, 2018 via email

@alfonsogarciacaro
Copy link
Member Author

alfonsogarciacaro commented May 16, 2018

Hi all again! We still need to implement the following methods in src/js/fable-core/List.fs (note it's an F# file though it'll be distributed in JS):

  • truncate
  • concat
  • findBack
  • findIndexBack
  • tryFindBack
  • tryFindIndexBack
  • indexed
  • mapFold
  • mapFoldBack
  • tryItem
  • unfold
  • splitAt

These are implemented but tests are not passing:

  • foldBack2
  • partition

These are a bit particular because Fable needs to inject IEqualityComparer as extra argument. I can do it myself but if you're brave enough you can take sort as an example:

  • distinct
  • distinctBy
  • groupBy
  • countBy
  • contains
  • except

I've put the failing tests in the QuickTest.fs file, so you should be able to recompile Fable + fable-core and run the tests just by typing:

cd src/tools
sh quicktest.sh --build-core

Any volunteers? ;) @valery-vitko @zaaack

@zaaack
Copy link
Contributor

zaaack commented May 16, 2018

I'll see what I can do this weekend.

@valery-vitko
Copy link
Contributor

I'm packed up full for the coming weeks, unfortunately. (And mostly with non-programming related stuff 😢) . Have fun without me, guys.

@Nhowka
Copy link
Contributor

Nhowka commented May 17, 2018

I fixed the partition and foldBack2 in #1405.
I'll try to do some others low-hanging functions, did truncate and indexed.

@Nhowka
Copy link
Contributor

Nhowka commented May 17, 2018

Implemented unfold, tryItem, splitAt and concat.

@alfonsogarciacaro
Copy link
Member Author

Hi there! There are still some missing methods in the list above. Is anyone working on them? If not, I'll give them a go 👍

@zaaack
Copy link
Contributor

zaaack commented May 22, 2018

@alfonsogarciacaro Sorry that I thought they are all taken, I'll take the rest.

PR: #1406

@Alxandr
Copy link
Contributor

Alxandr commented Jun 21, 2018

Question considering the new way of dealing with methods and interfaces; how will the following be handled?

type Base () =
  abstract M: unit -> string
  default __.M () = "base"

type A () =
  inherit Base ()
  member __.M () = "a" // NOTE: Not override, hides parent M

type B () =
  inherit Base ()
  override __.M () = "b"

let a = A ()
let b = B ()

printfn "%s" <| (a :> Base).M ()
printfn "%s" <| (b :> Base).M ()

@alfonsogarciacaro
Copy link
Member Author

Fable 2 implements own members as module functions and resolves the references at compile time. However virtual methods are attached to the JS prototype so they can be called even after upcasting the object. So this slightly expanded version of your code (to show how an object can call the parent member):

type Base () =
  abstract M: unit -> string
  default __.M () = "base"

type A () =
  inherit Base ()
  member __.M () = base.M() + " + a" // NOTE: Not override, hides parent M

type B () = 
  inherit Base ()
  override __.M () = base.M() + " + b"

let a = A ()
let b = B ()

printfn "A:        %s" <| a.M ()
printfn "A upcast: %s" <| (a :> Base).M ()
printfn "B:        %s" <| b.M ()
printfn "B upcast: %s" <| (b :> Base).M ()

Compiled with latest dotnet-fable 2.0.0-alpha becomes:

function Base() {}

function Base$$$$002Ector() {
  return this != null ? Base.call(this) : new Base();
}

Base.prototype.M = function () {
  return "base";
};

function A() {
  Base$$$$002Ector.call(this, null);
}

(0, _Types.inherits)(A, Base);

function A$$$$002Ector() {
  return this != null ? A.call(this) : new A();
}

function A$$M(__$$1) {
  return Base.prototype.M.call(this) + " + a";
}

function B() {
  Base$$$$002Ector.call(this, null);
}

(0, _Types.inherits)(B, Base);

function B$$$$002Ector() {
  return this != null ? B.call(this) : new B();
}

B.prototype.M = function () {
  return Base.prototype.M.call(this) + " + b";
};

const a = exports.a = A$$$$002Ector();
const b = exports.b = B$$$$002Ector();
(0, _String.toConsole)((0, _String.printf)("A:        %s"))(A$$M(a));
(0, _String.toConsole)((0, _String.printf)("A upcast: %s"))(a.M());
(0, _String.toConsole)((0, _String.printf)("B:        %s"))(b.M());
(0, _String.toConsole)((0, _String.printf)("B upcast: %s"))(b.M());

And prints:

A:        base + a
A upcast: base
B:        base + b
B upcast: base + b

@vbfox
Copy link
Contributor

vbfox commented Jun 22, 2018

@alfonsogarciacaro Will there be a way to disable method -> module conversion completely and get the clean Fable 1 result ?

@alfonsogarciacaro
Copy link
Member Author

@vbfox No, sorry. This would affect a lot how Fable 2 is working and unfortunately I don't have the resources to maintain compiler flags to generate different outputs :/

@vbfox
Copy link
Contributor

vbfox commented Jun 22, 2018

I completely understand the resource problem but it's sad as it breaks a lot of the compatibility between fable & the JS world at least when using classes anywhere :

  • Creating an npm package with fable for JS consumption
  • Using multiple languages to access fable generated class (typescript) or must have very strange bindings...

(Cases where external libs are doing method detection will also need interface creation but it's mostly good practice)

It's also going away from JS you can be proud of at least for classes - even if we don't care so much ;)

@alfonsogarciacaro
Copy link
Member Author

Yes, the generated code will be somewhat less beautiful in Fable 2. But there are other advantages (better tree shaking, static dispatch) that I hope will compensate this fact. About creating libraries in Fable for JS consumption, it will still be possible, though I haven't seen any actual example of this. And I already gave up using Typescript in my Fable projects because it always compiles whenever I try to import an .fs file ;)

The way to interop with JS should be interfaces. Fable will now generate object wrappers with no-mangled names for the interfaces so you can use them to send types to JS or other languages.

@Alxandr
Copy link
Contributor

Alxandr commented Jun 22, 2018

@alfonsogarciacaro I've really wanted to build libraries for consumption in JS made with Fable, but it was such a major pain that I gave up on it. It's the biggest reason I'm not using Fable for much more than I currently do. I've concluded it only works for building apps (whether web or not).

@alfonsogarciacaro
Copy link
Member Author

Thanks for the confirmation @Alxandr. So it was already painful in Fable 1, meaning Fable 2 cannot make it worse ;)

@Alxandr
Copy link
Contributor

Alxandr commented Jun 22, 2018

@alfonsogarciacaro sort of true. And I don't think moving to statics is going to matter (cause the interface objects are a pretty good solution). But I do think it might warrant talking about WHY it is massively painfull, because I think some of that might be resolved by fable 2. And I think the primary issue is the fact that fable-core lives on nuget and not npm (the js files that is). That means if I have a library and an app built in F#, I now have two copies of Map and List, etc. And you've also broken instanceof...

@alfonsogarciacaro
Copy link
Member Author

True. Actually we tried to solve that issue at one point (in Fable 0.7 I think, when fable-core and Fable itself were distributed through npm) but fable-core changed a lot and properly managing versions was a nigthmare (we also had to keep versions for different module systems). The situation is much better since Fable 1 were fable-core files are just injected by the compiler and we always have the files in sync. I don't think creating Fable libraries for npm that are meant to be also natively consumed by Fable is feasible. You either create a JS library (exposing a JS API surface and preferably bundling) or write a Fable/F# library that is distributed through Nuget.

The JS ecosystem is very broad and Fable opens many opportunities for F#. Unfortunately my reach is limited and I cannot take everything into account when making design decisions. So far, usually the introduction of Fable in different fields: Elmish web apps, React Native, Electron... has been thanks to contributors that created the app/libraries/tools and then we worked together to make the experience easier. But I haven't seen that yet for JS libraries created with Fable except for a couple of small samples.

@Alxandr
Copy link
Contributor

Alxandr commented Jun 22, 2018

Yeah, no I completely get that. My "be-all-end-all" solution would likely be a situation where you build fable-core (andy F#/js libraries) on a CI and publish to NuGet and NPM at the same time. That way you would also not need F# source files on nuget (you would only have build-metadata, but you would need the npm import name). There is a lot of pieces that needs to come together for this to work though, and it might require storing a bunch of metadata in the nuget package (mapping to JS function names that was generated).

@Alxandr
Copy link
Contributor

Alxandr commented Jun 23, 2018

So, I've been thinking a lot about the new Fable output today and I've come up with a few maybe-issues that I would like to bring up in case they haven't been considered:

Classes vs functions

I'm not a huge fan of classes in JS myself, however, they do serve one important purpose that cannot be achieved with functions (as far as I'm aware), namely extending native classes (like Map or Array). Now, I'm assuming the main reasons for using functions instead of classes is the fact that you need to deal with multiple constructors, but I'm reasonably certain that can be solved other ways (I'll look a bit more at that again tomorrow). Now, just to be clear, I'm not talking about going back to attaching everything to the prototype. I'm just talking about making the primary (or synthetic) constructor be a class. As for the ability to call the constructors without new, I think that's a rather moot point, considering the fact that F# is a typed language, and Fable should be able to statically figure out whether it needs to emit new or not based on what function it's calling.

Interface casting and type checks

I've seen code that deals with type-hierarchies nicely, and allows for type-checking and calling virtual members, which is all quite nice, but how does the new output work with interfaces in conjecture with type-checks? Take for instance the following code:

type IFoo =
  abstract Foo: string

type IBar =
  abstract Bar: string

type FooBar () =
  interface IFoo with
    member __.Foo = "foo"

  interface IBar with
    member __.Bar = "bar"

let check (t: bool) =
  if not t then failwithf "test failed"

let test () =
  let foobar = FooBar ()
  let foo = foobar :> IFoo
  let bar = foobar :> IBar
  let obj = foobar :> obj
  
  check (foo :? FooBar)
  check (bar :? FooBar)
  check (foo :? IBar)
  check (bar :? IFoo)
  check (obj :? IFoo)
  check (obj :? IBar)
  ()

@alfonsogarciacaro
Copy link
Member Author

We can close this issue now, thanks a lot everybody for your help in releasing Fable 2!

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

No branches or pull requests

10 participants