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

Coffeescript to ES6 #492

Closed
GianlucaGuarini opened this issue Dec 11, 2014 · 32 comments
Closed

Coffeescript to ES6 #492

GianlucaGuarini opened this issue Dec 11, 2014 · 32 comments

Comments

@GianlucaGuarini
Copy link

What do you think if we move the project forward migrating the current coffeescript code to javascript next? I think we could transpile it using https://6to5.github.io/

@raimohanska
Copy link
Contributor

That might be a good idea. Been thinking about it too. Wanna take a stab at it?

@GianlucaGuarini
Copy link
Author

I'd be really happy to start the migration to ES6 (hopefully fixing the memory issues). What about the build step? Do you prefer grunt or do we want to switch to gulp?

@raimohanska
Copy link
Contributor

I have no preference with regard to build tools, as long as we are able to generate the same build outputs.

@lautis
Copy link
Member

lautis commented Dec 11, 2014

You'll likely be disappointed about the memory effect of converting the code to use ES6. It's not like CoffeeScript code would inherently use more memory as you can compile it to JS. Similar refactorings that might lead to lower memory consumption could be applied to the CoffeeScript source as well.

There's even one gotcha in ES6 if you use the arrow syntax for functions as it automatically binds to this. Assuming the explicit binding isn't needed (and this in the function is always the same as in the scope) the compiled ES5 would consume more memory.

Of course, if you run ES6 natively (node?), more optimisations might be applied.

For other reasons ES6 does sound promising 👍

@GianlucaGuarini
Copy link
Author

@lautis migrating a library like Baconjs to es6 is quite a big task but if it doesn't help improving the memory issue it's just wasted time for me, I would step back suggesting a code refactor.
@raimohanska has cleverly admitted that Baconjs has still "a lot of fat" this means that if you've planned to change a lot of the core of the library it doesn't make sense to me to start migrating the current code to ES6, but it would be better to start refactoring it using pure javascript syntax

@lautis
Copy link
Member

lautis commented Dec 11, 2014

I wouldn't call it wasted time. You'd definitely have a lot deeper understanding how Bacon could be optimised. However, that would require a bit more than doing a mechanical transformation.

At least to me this seems like a hard way to improve memory usage. If you find CoffeeScript incomprehensible, the pure JS codebase might be more pleasant to work with.

A quick way to get the JS migration done, would be to take the compiled JS output and clean the worst CoffeeScript idiosyncrasies.

@GianlucaGuarini
Copy link
Author

have you guys an irc channel? May we discuss deeper about that in a chat? https://gitter.im/?

@raimohanska
Copy link
Contributor

Ok then, let's try the Gitter chat... h

https://gitter.im/baconjs/bacon.js

@sebmck
Copy link

sebmck commented Dec 15, 2014

@lautis 6to5 doesn't bind functions to this, it aliases the contained this with the upper context (just like CoffeeScript arrow functions). So if you don't use this inside arrow functions then you'll face no perf/memory decrease.

@sebmck
Copy link

sebmck commented Dec 15, 2014

There's also a CoffeeScript to ES6 transpiler on the 6to5 roadmap but I can't say for sure when that'll be available, definently some time in the near future though.

@AlexGalays
Copy link

+1 for a rewrite to ES6/ES5, or even vanilla JS + sweet.js macros.
CS is quite alien for many JS devs.

@GianlucaGuarini
Copy link
Author

I have checked the Baconjs generated from coffeescript and the other one coming from my migration/es6 fork. Here you can run the benchmark by yourself http://jsperf.com/bacon-coffeescript-vs-bacon-es6 and check the results
It looks that the es6 output is faster on firefox and slower on chrome. In any case I think I need someone who helps me during the migration translating the whole library entirely to es6.

@lautis
Copy link
Member

lautis commented Jan 8, 2015

With Node 0.10 performance seems pretty badly reduced on the ES6 branch, but Node 0.10 based on quite old Chrome. Replacing this.subscribe.bind(this) with a closured version might improve that as Function.prototype.bind is pretty slow at least on older Chromes.

@sebmck
Copy link

sebmck commented Jan 8, 2015

@lautis That benchmark is incorrect since var test = function(){ return this }; in the self test should be var test = function(){ return self; };. Although self will still be fastest.

@lautis
Copy link
Member

lautis commented Jan 8, 2015

I changed the link to point to a later revision that seems to be a bit less wrong.

@GianlucaGuarini
Copy link
Author

Ok I will update the branch using replacing the native bind functions to get faster performances also on Chrome

@GianlucaGuarini
Copy link
Author

@lautis changing the native bind to the wrapped one the es6 branch is a bit faster than the master. I hope that with my next updates, splitting the source code into smaller modules it will become also a bit more readable.

@GianlucaGuarini
Copy link
Author

I am sorry guys but I must give up migrating Baconjs to es6.
Here a list of reasons:

  • There are too many weekly changes to the original Coffeescript source code even on the project scaffolding (see assemble.js) deprecating the code I am working on too quickly
  • 6to5 became on any new release more and more unstable/unusable relying on external polyfills and polluting the output code, a library like Bacon cannot include several polyfills/runtime in its source code
  • I am the only one working on this branch I didn't get any help working on the migration and it became quickly too much work
  • there is too much Coffeescript here. The grunt tasks, the tests and the source code are all written in Coffeescript so probably you don't really want/need to switch to pure javascript just for the glam

I learned a lot reading the Baconjs source code but I've also understood that a project like Rxjs fits more to my needs.

@sebmck
Copy link

sebmck commented Jan 13, 2015

@GianlucaGuarini

6to5 became on any new release more and more unstable/unusable relying on external polyfills and polluting the output code, a library like Bacon cannot include several polyfills/runtime in its source code

That's factually incorrect. 6to5 has actually become less reliant on polyfill and shim methods. The output code has been coming less and less verbose and with features such as loose mode that outputs simpler and faster code but is less spec compliant I take extreme issue with those kind of statements.

There are only a few features that require Array.from and Symbol polyfills see the caveats and that's only to support all iterables for spec compliancy. There's even a mention in the FAQ about how you can use these features without requiring a global polluting polyfill.

@GianlucaGuarini
Copy link
Author

@sebmck don't you think that for something simple like:

// es6
for (var item of array) {
//...
}

an output like this (+ a huge Symbol polyfill ) :

// 6to5 output
for (var _iterator = array[Symbol.iterator](), _step; !(_step = _iterator.next()).done;) {
  var item = _step.value;
}

is a bit too much?
It cannot compete against the coffeescript output

var _i, _len;
for (_i = 0, _len = array.length; _i < _len; _i++) {
}

@jamiebuilds
Copy link

Hey @GianlucaGuarini, I can imagine why that would be frustrating. However, it needs to be the compiled 6to5 output in order to properly follow the ES6 specification. This isn't a stability issue, it's by design.

We've been looking into how people can opt-in to a simpler output which ignores various parts of the spec. This is what @sebmck meant by loose mode, and it's likely what you are looking for.

@sebmck
Copy link

sebmck commented Jan 13, 2015

@GianlucaGuarini ES6 for...of is not equivalent to the CoffeeScript for...of. In ES6 it's to iterate over an Iterator, which can be a string, array, or an object that defines how it's supposed to be iterated over. Your example is comparing CoffeeScript for...in with ES6 for...of which is functionally different.

@GianlucaGuarini
Copy link
Author

@sebmck
in case of an array it's clear that the iteration is over its items so probably the output could be simpler, the Symbol must be probably output mostly for Sets and Maps. In any case In the previous 6to5 releases this problem was not there.
@thejameskyle
The problem is that no library can be developed including polyfills directly in the source code if before it was working even on IE6 as standalone script.

@lautis
Copy link
Member

lautis commented Jan 13, 2015

@GianlucaGuarini that's unfortunate 😿 (and understandable, porting over patches isn't exactly the most exciting job). We'll need to be more focused if we want to get this done.

@jamiebuilds
Copy link

in case of an array it's clear that the iteration is over its items so probably the output could be simpler

That would only work when we know a value is an array literal.

function(items) {
  for (let item of items) {
    // no way of knowing items is an array.
  }
}

@sebmck
Copy link

sebmck commented Jan 13, 2015

@sebmck

in case of an array it's clear that the iteration is over its items so probably the output could be simpler

We can't keep track of types unless they're inlined as literals.

the Symbol must be probably output mostly for Sets and Maps. In any case In the previous 6to5 releases this problem was not there.

It's been present since the inclusion of the for of transformer. See babel/babel@b0cfbb2.

As I said before, 6to5 has ways to facilitate the inclusion of these methods and native types without polluting the global scope with a polyfill. See the FAQ and coreAliasing.

@phadej
Copy link
Member

phadej commented Jan 14, 2015

Let's see if the assemble.js could be changed to accept both coffeescript and es6 parts. Then we don't need to do everything at once. And we can use work @GianlucaGuarini has already done!

@phadej
Copy link
Member

phadej commented Jan 14, 2015

Here is working PoC: #526

@GianlucaGuarini
Copy link
Author

@sebmck

It's been present since the inclusion of the for of transformer. See babel/babel@b0cfbb2.

Oups sorry :D then probably I have imagined that

BTW It's clear that 6to5 it's an awesome tool, but I am not sure in this case It can compete against the clear and dry coffeescript source code used for the Baconjs library

@sebmck
Copy link

sebmck commented Jan 14, 2015

@GianlucaGuarini By default, definently. Loose mode however will be better, if not equivalently fast.

EDIT fixed a link

@GianlucaGuarini
Copy link
Author

looking forward to try it out thanks for your support

@raimohanska
Copy link
Contributor

Closing for now, until some progress seen, if any...

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

7 participants