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

TypeScript bindings #232

Closed
fdcastel opened this issue Jul 6, 2016 · 33 comments
Closed

TypeScript bindings #232

fdcastel opened this issue Jul 6, 2016 · 33 comments

Comments

@fdcastel
Copy link
Contributor

fdcastel commented Jul 6, 2016

To integrate Fable-generated code with an ES2015 project is a breeze. Plain simple. It just works.

However, the same cannot be said for TypeScript projects.

"Typings... Typings everywhere!"
-- The Rime of the Ancient TypeScripter

  1. fable-core requires a .d.ts file. Has anyone already made one? (I can work on it)

  2. The same for every .js file created by Fable. Could the compiler generate it automatically for us? How hard it will be?

@alfonsogarciacaro
Copy link
Member

  1. Hmm, I don't think anybody has started a .d.ts for fable-core (definitely not me 😉) Actually at the beginning I was thinking to release fable-core also as a JS library (that's why List methods are both instance and static) but I don't think it's feasible now because a) besides List, most methods are static which is not idiomatic in JS (e.g., Observable module vs RxJS) and b) there're many holes in fable-core: methods that are missing because they're inlined in the Replacements module. Do you want to use the .d.ts to call fable-core from TypeScript? Are you doing it now from JS?
  2. Some people already requested automatic d.ts generation from F# files and I've thought about it a couple of times, but haven't started any work on it yet. I guess it shouldn't be too difficult though, at least much easier than going the opposite direction from TypeScript (and all its weird type definitions) to F#. Only a bit of care must be taken for naming as I recently allowed root members with non-valid JS identifiers.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 7, 2016

Do you want to use the .d.ts to call fable-core from TypeScript?

Exactly.

Are you doing it now from JS?

Yes. I need it in some cases when integrating existing JS code with Fable-generated code (e.g. to create a List and pass it as an argument to a F#-translated function).

In an ES2015 project this is very straightforward. A simple import * from 'fable-core/es2015' and we're done.

But for a TypeScript project this also requires a .d.ts file. Without one the compiler doesn't recognize the imported file as a module at all.

For now I did some dummy declarations only for the types I need, e.g.:

// es2015.d.ts
export var Array: any;
export var List: any;

just to keep the compiler happy.

I will be playing a bit more with it. Let's see what happens :)

@alfonsogarciacaro
Copy link
Member

I thought the TypeScript was able to read an ES2015 module even without the type definitions, it seems I was wrong 😉

I wrote a very simple script to read the (static) members of fable-core modules for a double check in the Replacements module, maybe something similar could be used to generate the d.ts.

Another solution is, like import/core/es2015.js is used now as the basis to generate the versions with other module systems, we could use a .ts file instead with the type definitions for each function that can be read from TypeScript. What do you think?

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 7, 2016

I thought the TypeScript was able to read an ES2015 module even without the type definitions

Me too! Until you use it and start to see dozens of "cannot find module ..." 😅

maybe something similar could be used to generate the d.ts.

It will solve only part of the problem. Any automated solution based on a .js file could at most generate a "skeleton" file. It will yet require some human intervention to add the types. E.g., in the line

export var List = function List(head, tail)

We have no (automated) means to know about the expected type of head and tails arguments for this function. Nor about its return value.

we could use a .ts file instead with the type definitions for each function

Hm... Yes! This would indeed be better. Using a .ts file carries "more expressiveness" than the current .js (ES2015) one. It would also improve fable-core documentation in some way.

HOWEVER... it will make the build system more complex (it will require TypeScript to build Fable) and also the maintenance of fable-core. Any modification in fable-core would require a TypeScript connoisseur to make the change.

Sincerely: unless you are very comfortable with TypeScript I would recommend against this and just keep a separate .d.ts file for now. It is a fantastic language, but it can give you wrinkles sometimes... 😄

@alfonsogarciacaro
Copy link
Member

Not a very good practice but the current CoreLib build target already requires Babel to be installed globally so I guess replacing babel with tsc wouldn't complicate things that much.

I haven't worked that much with TypeScript but I'm comfortable with the language and given the fact that advanced F# users (that is, potential Fable contributors) usually know well Microsoft technologies, probably they can handle TypeScript. Also, maintaining a different .d.ts has its own complications: you still need to know TypeScript anyways and you must be careful to keep it in sync with the JS code.

In conclusion, I'd be in favour of writing fable-core in Typescript (shouldn't be much harder than adding the type annotations to the function arguments) but unfortunately I don't have time for that at the moment. If you can start working on that and send a PR it'll be fantastic!

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 7, 2016

If you can start working on that and send a PR it'll be fantastic!

Already working on it ;)

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 7, 2016

Please see #236. I did a small declaration (for List functions only) just to assess the work needed.

The declarations are far from the ideal. They can be much more strict (using generics, less "any" types, etc). I made them quickly, just to make it work.

I will work now in the changes needed to compile es2015.js using TypeScript instead of Babel. Then we can compare both options (keeping a .d.ts separately vs build it entirely from TypeScript).

@alfonsogarciacaro
Copy link
Member

I panic whenever two files must be kept in sync without anything that forces that in the build process 😉 so I'd still be in favour of writing fable-core directly in TypeScript.

About generating .d.ts automatically, maybe we can use this Babel plugin. I just had a quick look so I'm not sure if it's usable for us, but in any case it cannot be used at the moment as Fable doesn't output type annotations. We can do that but we'll have to modify the Fable AST as it's very loosely typed and much of the type information is lost at the moment when passing from F# to Fable.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 7, 2016

I surely understand ;)

I'm working on a "mini-fable-core.ts" right now. I believe I can push a first version soon. Then we can discuss the possible options (classes, interfaces, namespaces, etc.)

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 8, 2016

Initial proof-of-concept to port es2015.js to TypeScript ready.

It is far from complete. List and (partial) Seq classes only.

To build: From project root folder, run:

tsc src/fable/Fable.Core/es2015-mini.ts --target ES2015 --outDir src/fable/Fable.Core --declaration

This will create

src/fable/Fable.Core/es2015-mini.js
src/fable/Fable.Core/es2015-mini.d.ts 

Please, compare the generated es2015-mini.js file with the current es2015.js and tell me what you think (any problems? Something strange? Anything that could cause problems later?).

BTW: I spent the last hour trying to compile the generated .js file with Babel (to make it more similar to the current es2015.js) and... I failed miserably 😄 -- So... I'll let this to the pros. Currently, my experience with Babel transformations is a very round number nearing to zero. ;)

Also: please note that the TSC compiler target must be ES2015. Any other target and the compiler will not recognize iterators and generators. This will hopefully change in TypeScript 2.0. But, for now, we will probably need Babel if the output of TSC isn't enough.

If no problems arise, I can proceed to the conversion of the remaining es2015.js file. It will probably take several days but it surely can be done.

@alfonsogarciacaro
Copy link
Member

When compiling with Babel you need to pass the plugins you want to use, and they can be a lot to convert from ES2015 to ES5 for example (that's why they usually use presets). In the FableCore build target only modules are transformed: note there're actually no other ES2015 features in es2015.js.

Oh my, TypeScript cannot compile generators to ES5? Come'n Microsoft! Anyways, as said above, iterators are created with no ES2015 syntax in es2015.js so if you just copy the code as is it should work.

It'd be nice if you try to keep as many lines as common between es2015.js and the typescript code (basically the body of the functions). This way we can compare the files and be sure there've been no typos, etc.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 8, 2016

Yeah! I'm aware of Babel plugins and presets. But I couldn't find a combination to output the .js like the current one (i tried with the "ES2015" preset and also with some combinations of plugins -- like the ones used in Fable source). It was probably due the sleep deprivation 😄 I will try again, later.

It'd be nice if you try to keep as many lines as common between es2015.js and the typescript code (basically the body of the functions).

Agreed! I started this way. However, after several revisions of the code I did choose to make some minor "syntatic" changes (like changing function delegates to lambdas, to use const/let instead of var, etc). All changes which should not affect the output after being transpiled to ES5.

My idea is to write the source in the most TypeScript-way, to enjoy the benefits of the language, but which -- after the transpile -- can get to something very similar to the current fable-core.js (ideally, the same). That's what I failed to got with Babel (but, as I said, I will try again).

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Found it!

The problem wasn't the combination of Babel plugins but its options.

The loose option in transform-es2015-classes plugin is exactly what I was looking for!

TypeScript code:

export class Seq<T> {
  static fold<T, ST>(f: (previousValue: ST, currentValue: T, currentIndex?: number) => ST, acc: ST, xs: Iterable<T>): ST {
    if (Array.isArray(xs) || ArrayBuffer.isView(xs)) {
      return (xs as Array<T>).reduce(f, acc);
    } else {
      let cur: IteratorResult<T> = null;
      for (let i = 0, iter = xs[Symbol.iterator](); ; i++) {
        cur = iter.next();
        if (cur.done) {
          break;
        }
        acc = f(acc, cur.value, i);
      }
      return acc;
    }
  }

  static foldBack<T, ST>(f: (currentValue: T, previousValue: ST, currentIndex?: number) => ST, xs: Iterable<T>, acc: ST): ST {
    const arr = Array.isArray(xs) || ArrayBuffer.isView(xs) ? xs as Array<T> : Array.from(xs);
    for (let i = arr.length - 1; i >= 0; i--) {
      acc = f(arr[i], acc, i);
    }
    return acc;
  }
}

TSC |> Babel --preset ES2015 with ["transform-es2015-classes", { "loose": true }]:

var Seq = exports.Seq = function () {
    function Seq() {
        _classCallCheck(this, Seq);
    }

    Seq.fold = function fold(f, acc, xs) {
        if (Array.isArray(xs) || ArrayBuffer.isView(xs)) {
            return xs.reduce(f, acc);
        } else {
            var cur = null;
            for (var i = 0, iter = xs[Symbol.iterator]();; i++) {
                cur = iter.next();
                if (cur.done) {
                    break;
                }
                acc = f(acc, cur.value, i);
            }
            return acc;
        }
    };

    Seq.foldBack = function foldBack(f, xs, acc) {
        var arr = Array.isArray(xs) || ArrayBuffer.isView(xs) ? xs : Array.from(xs);
        for (var i = arr.length - 1; i >= 0; i--) {
            acc = f(arr[i], acc, i);
        }
        return acc;
    };

    return Seq;
}();

Current fable-core.js:

  var Seq = exports.Seq = {};
  // ...
  Seq.fold = function (f, acc, xs) {
    if (Array.isArray(xs) || ArrayBuffer.isView(xs)) {
      return xs.reduce(f, acc);
    } else {
      for (var i = 0, cur = null, iter = xs[Symbol.iterator]();; i++) {
        cur = iter.next();
        if (cur.done) {
          break;
        }
        acc = f(acc, cur.value, i);
      }
      return acc;
    }
  };
  Seq.foldBack = function (f, xs, acc) {
    var ar = Array.isArray(xs) || ArrayBuffer.isView(xs) ? xs : Array.from(xs);
    for (var i = ar.length - 1; i >= 0; i--) {
      acc = f(ar[i], acc, i);
    }
    return acc;
  };
  // ...

Now THAT'S what I'm talking about. 😉

As you can see, even making heavy use of TypeScript features (classes, generic types, etc) we can yet produce the same output which ES2015.js |> Babel does today to generate fable-core.js.

I will push some more changes soon, including this process in the build script. Then we can better assess the results.

@alfonsogarciacaro
Copy link
Member

Nice! Now I remember there was something about that in Babel Discuss 😅

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Updated #240

build FableCore now compiles TypeScript sources. You can compare the output (fable-core.tsc.js, yet very incomplete) with the official fable-core.js.

Important: you'll need to run npm install babel-preset-es2015 from src/fable/Fable.Core folder. I had to put a .babelrc into this folder since I can't find a way to pass plugin options via Babel CLI.
This is temporary and will be fixed in the future.

I will add some more classes into fable-core.tsc.ts soon to get a better picture of the change. For now, it seems very promising. I hope we don't hit a bump 😅

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

BTW: I fixed a minor glitch when running build FableCore on Windows. You may wish to cherry-pick just this one fsprojects-archive/zzarchive-Fable@0e2c1fc into master. ✌️

@alfonsogarciacaro
Copy link
Member

Tomas also found this problem and sent a PR ;) I'm on Windows now too and I'm trying to fix things again to be easily open Fable on VS but I've found a problem that I'm trying to fix.

About passing Babel plugins via CLI, it's possible but they're always resolved locally. My dirty fix for that was to pass the plugins installed in build/fable.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Hahahah. No. Forget about it. @tpetricek won the race with #241 😂

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

About passing Babel plugins via CLI, it's possible

Sure! However, the problem isn't the PLUGIN but its OPTIONS 😉. Please see:

{
  "presets": [
    "es2015"
  ],
  "plugins": [
    "transform-es2015-modules-umd",
    [
      "transform-es2015-classes",
      {
        "loose": true
      }
    ]
  ]
}

I tried to keep everything like you did, but I couldn't find a way to pass that "loose": true via Babel cli. 😢

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Two PR for the same problem in the same day and now two comments about it at almost the same time. Must be some record. 😅

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Latest changes merged in #240

Everything like before. Same instructions apply. Just replace src/fable/Fable.Core with src/fable/Fable.Core/npm where applicable :)

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 9, 2016

Added some more types in #240. This time choosing to keep the generated fable-core.tsc.ts as similar as possible to fable-core.js (mostly "copy & paste" work) instead of trying to use Typescript capabilities to their fullest extent.

@alfonsogarciacaro
Copy link
Member

Thanks! I'll have a look later... I think I need to take a rest from Fable this weekend ;) Sorry for so many changes in the build process, I'm trying to find the best structure so it's easier for other people to contribute. Hopefully it should be fine now 👍

@fdcastel
Copy link
Contributor Author

New push in #240.

Full implementation of fable-core.js in TypeScript ready! Build now generates fable-core.js from TypeScript sources instead es2015.js.

(the original code, built from es2015.js, is compiled into fable-core.babel.js for comparisons)

Problem: The tests are failing with an weird error ("TypeError: Cannot assign to read only property 'length' of function Seq()") albeit the body of Seq.length function is the same from the original code. It seems like a scope problem. I'll come back to it tomorrow. Just pushing it to report.

Have a great weekend!

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 10, 2016

Latest update in #240 -- All tests passing now! 🎉

About the length problem: A static 'length' method causes problems in JavaScript. In the current version TypeScript compiles the code but it fails in runtime. In the future the compiler will probably give a compile-time error. So I changed the method name to _length and put a replacement to it in Fable compiler.

I also did some minor improvements in some tests to catch a few quirks I was getting.

From this one I believe it would be possible to start using TypeScript sources to generate the official fable-core.js. There is yet a ton of things to improve in the sources (since the focus until now was to keep the output the most similar possible to the current es2015.js output). But I will wait for your approval before to start them.

If you choose to merge this, please consider the following:

  • The build script is building both versions: TypeScript and ES2015.
    • The latter should be removed in the final merge Bad idea. I changed my mind. See below.
  • I would recommend to rename fable-core.tsc.ts to fable-core.ts.
  • The TypeScript output requires a Babel transformation which currently uses a .babelrc file in src/fable/Fable.Core/npm and needs a npm install babel-preset-es2015 in this same folder. All of this, of course, very far from ideal. It will require some restructuring here.
    • Alternatively: this can be entirely removed if we give up on the use of loose option in transform-es2015-classes. This way we could just go back to passing babel arguments via command line and use node_modules from /build/fable folder.

@alfonsogarciacaro
Copy link
Member

Fantastic work! I'll check it more carefully but it's looking good so far 👍

Though, to be honest, I feel a bit uneasy if you had to tweak the tests to make them pass, as this can reveal fails in the compilation process. What were exactly the quirks you were getting?

Also, about length, that's unfortunate, but since the underscore is usually used for intended private members in JS, I would change the name to something like count, what do you think?

@fdcastel
Copy link
Contributor Author

I feel a bit uneasy if you had to tweak the tests to make them pass,

Don't worry 😉. I just expanded them in some cases (like in .collect tests) or added more checks to pinpoint what are going wrong in my version (like in TimeSpan constructors).

These changes were made only to help me to find problems in my port. Not to compromise the tests 😅. You can test them against master just by cherrypicking fsprojects-archive/zzarchive-Fable@db0a0e5

Also, about length, (...) I would change the name to something like count,

Perfect! 👍

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 11, 2016

On a second thought, we should not remove the es2015.js version.

Doing this would be in detriment of anyone who wants to integrate existing ES2015 code with Fable (like, for example, myself 😄).

I propose we keep the files as follow:

  • fable-core.ts (source)
    • es2015.js (Created from fable-core.ts |> tsc --target ES2015)
      • fable-core.js (Created from es2015.js |> babel --plugins ...umd)
        • fable-core.min.js (Created from fable-core.js |> uglifyjs)
      • commonjs.js (Created from es2015.js |> babel --plugins ...commonjs)

@alfonsogarciacaro
Copy link
Member

Hmmm, still I'd prefer if you add to the tests but don't replace anything. This is because so far I had many surprises of apparently trivial changes in the tests that revealed subtle problems with the compilation. I'll add some comments to you PR but in general I sleep better if I don't see red in the tests, only green... both for the results and the git diffs 😉

@fdcastel
Copy link
Contributor Author

I sleep better if I don't see red in the tests, only green... both for the results and the git diffs

Hahaha... Got it! 😄

You are absolutely right. I did remove the code under the assumption my new code would cover the old one. I'm fixing this right now.

@alfonsogarciacaro
Copy link
Member

Thanks! About the file structure, I agree with that 👍

Only thing is: at what point are ES2015 features (besides modules) transformed? When converting from fable-core.ts to es2015.js or from es2015.js to fable-core.js. I ask this because the purpose of es2015.js was originally to be used with Webpack 2 for tree shaking but still being compatible with environments not compliant with ES2015 syntax.

@fdcastel
Copy link
Contributor Author

fdcastel commented Jul 11, 2016

or from es2015.js to fable-core.js.

This one.

TypeScript are being told to target ES2015. In this target, as a rule of thumb, you can think of it more as a "types enforcer" than a "coding generator".

The "heavier" transformations (from ES2015 to ES5) are made by Babel plugins.

@alfonsogarciacaro
Copy link
Member

Closing the issue as the PR is already merged. Please feel free to send additional PRs to improve the definitions.

Thanks a lot for your contribution!

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

2 participants