HELP WANTED: ES6 Conversion Effort

[bigtimebuddy]

Hello pixi devs!

Thanks to this PR #2936 (shoutout to @leonardo-silva!) we have begun an effort to convert the code-base of Pixi.js to ES6. The purpose of this upgrade is to future-proof as well as make Pixi.js more maintainable. While the source will be ES6, we will continue to build to ES5-compatible JavaScript using Babel. But hopefully in the future we'll be able to provide an ES6+ build.

Usually these types of changes are very challenging and difficult to pull off because of how distruptive they are to existing PRs and development, so ideally we'd like to get to stability as quickly as possible. So, we need your help!

There are a couple things that would be really useful if you're looking to help:

• We have setup a dev-es6 branch and welcome PRs to that branch for those proficient with ES6. Particularly, looking for additional PRs to add more use of const, fat arrows functions, and static getters.
• We need help performance testing the Babel build to make sure we have not compromised any of the amazing Pixi-speed we have all come to love.
• We could use help smoke-testing the latest build on this branch (see below for build links). Please add this to your v4 projects and post if you find any issues.
• We could use help smoke-testing the documentation to make sure jsdoc still plays well with ES6 (see link below)

ES6 Builds
http://s3-eu-west-1.amazonaws.com/pixi.js/dev-es6/pixi.js
http://s3-eu-west-1.amazonaws.com/pixi.js/dev-es6/pixi.min.js

Documentation
http://s3-eu-west-1.amazonaws.com/pixi.js/dev-es6/docs/index.html

[themoonrat]

Which route do you want to go down with let and const? Default to const, and only use let for properties that should have their reference changed
Or
Default to let, and only use const as more of a hint to the developer that they, I'm serious, don't change this.

[bigtimebuddy]

The former option. Default to using const. I converted all internal vars to let and fixed obvious scoping issues and disallowed the use of var with jshint. Needs another pass with const.

[englercj]

Things I would recommend:

• Use babel-preset-es2015-loose, I had poor perf with just babel-preset-es2015 alone.
• Switch to eslint, it has better ES6 support and is just better than jshint in general. Here is a good starting point for pixi's style. It will also complain about places you could use const but used let. Makes it easy to find all the places you need to fix for const.
• Use the latest master of jsdoc, it has a lot of ES6 fixes in there that haven't been released yet.
• Switch to webpack. More and more of our users are using this, and I think it fits our use-case better.

[bigtimebuddy]

Thanks @englercj. Good list.

[RanzQ]

@englercj The babel-preset-es2015-loose has a deprecation warning, did you try babel-preset-es2015 with the option {"loose": true} like it suggests?

I also prefer webpack to browserify, a couple of useful links:
https://github.com/webpack/webpack/tree/master/examples/multi-part-library
http://krasimirtsonev.com/blog/article/javascript-library-starter-using-webpack-es6

[bigtimebuddy]

My preferences is to wait on webpack and not pile that on. Potentially for another smaller PR. That change represents a build process change but not necessarily improvement to ES6. Also, would need to make sure sourcemaps, headers etc. work. I understand how it's better but let's wait for that for now.

[englercj]

The babel-preset-es2015-loose has a deprecation warning, did you try babel-preset-es2015 with the option {"loose": true} like it suggests?

Ha, that was just added 2 weeks ago. I haven't tried that because it didn't exist at the time I started using it.

[ghost]

Hi everyone,

Just a quick shout to echo Chad's thought about the loose mode, and add my two cents here.
As we discussed with @GoodBoyDigital before we have to be really careful about performance, so I think the loose mode is a good starting point.

Also, I'm not sure how up-to-date this table is : https://kpdecker.github.io/six-speed/ do you guys know?

If it still is then we have to be careful not to go ES2015 madness and convert things that don't need to be, i.e: if for-of still brings down the performance as it says on the table, we shouldn't use it when iterating scene graph children.

[RanzQ]

Babel should now optimize for-of for arrays (see: Optimization), those tests seem to be quite old.

Anyway, @alvinsight you're right, everything doesn't need to be ES2015.

[GoodBoyDigital]

Good list @englercj !

Also agreed with @alvinsight. We are already seeing much cleaner code with es6 syntax so winning all round :)

Every other decision should be based on perf - 'Does it look better but run slower - don't do it'

Array looping is a good example - there's no need to replace stuff that is a fast and readable already just because we can.

Also yes to webpack! @bigtimebuddy is correct, that should be part two of this shift to es6.

[ghost]

Agreed!
It's so much nicer to finally be able to read and write class Sprite extends Container !

[bigtimebuddy]

Updated

• Replaced jshint with eslint (and fixed linting errors, eslint for the win!)
• Using loose: true with babel-preset-es2015

[GoodBoyDigital]

Evening all!

Hit a little shnag with our mixins..

Object.assign(
    core.DisplayObject.prototype,
    require('./interactiveTarget')
);

The above not currently working on in the ES6 branch so things like interaction are busted.

Is there a nice way to do this with es6 classes?

Answers on a postcard please!

[GoodBoyDigital]

Ok I was right the first time - the above code is not working currently.. (Can I blame this on the fact its Friday night?)

[GoodBoyDigital]

I guess this is where composition shines!

[englercj]

Hmm, what isn't working? This worked for me:

class MyThing {}
Object.assign(MyThing.prototype, { newFn: function () { console.log('mix'); } });
var mything = new MyThing();
mything.newFn(); // logs: "mix"

Copy paste that into chrome console, and seems to work fine.

[GoodBoyDigital]

Interesting! Currently interaction is not working and interactive properties seem to be missing. Maybe another reason? Will keep digging...

[GoodBoyDigital]

Ah ha! Found it

Object.assign(
    core.DisplayObject.prototype,
    require('./interactiveTarget') <--- this is a require!
);

[englercj]

import interactiveTarget from './interactiveTarget';

Object.assign(
    core.DisplayObject.prototype,
    interactiveTarget
);

Maybe?

[GoodBoyDigital]

yup!

[GoodBoyDigital]

PR 1 here: #2960

It fixes a few bits like the mixins and text.

Tomorrow I will look at filters...

Looking really good though chaps!

[bigtimebuddy]

Good work! Yah, using require() like that would return an object with a single default key containing everything else you expected.

[GoodBoyDigital]

Filters looking good now! Ran this on a current project I'm working on that's pretty complex - and everything seems to work 100%!

Might test it out on a few other games but should be good to merge pretty soon!

@bigtimebuddy have you tried this build with pixi-animate?

[GoodBoyDigital]

Throwing this out there.. traceur seems faster than babel?
Worth investigating?

https://kpdecker.github.io/six-speed/

[englercj]

Those tests are old, and not run with babel loose. Besides, you could also argue that in many cases typescript is faster than both :)

[endel]

I'll suggest to you guys to consider using TypeScript as part of this major rewrite/adaptation. Just some points about TypeScript:

• Using type system in JavaScript will become the new standard, it's just a matter of time.
• It IS JavaScript. No difference in syntax besides the type system.
• Improves code quality. You'll possibly find a considerable amount of code that needs to be slightly restructured to match the types in the right places.
• Increase pull-request quality check - if there's some type issue, the patch simply cannot be merged.

I know this is not an easy chore, but I believe it can bring major benefits for the code base and the community.
Cheers!

[ivanpopelyshev]

@endel I'm moving pixi-spine to typescript, there will be demos of TS plugins for pixi.

[photonstorm]

@endel just curious, but has TS resolved the issue it had when I last looked at it: that it's an all or nothing situation when it comes to the output, i.e. everything is transpiled back to ES5, or nothing is, based on the target?

I remember it didn't use polyfills at all. So if you want a single code-base to run on a wide range of browsers, from cutting edge down to legacy, you still have to target ES5, and all of the nice new features, that modern browsers now support natively, get ignored because it downgrades to ES5 anyway? Or is it still the case where you need to use an ES6 polyfill on-top of TS output to cover all bases?

[englercj]

that it's an all or nothing situation when it comes to the output, i.e. everything is transpiled back to ES5, or nothing is, based on the target?

Not sure what you mean by this. If you target ES5, it transpiles the syntax to ES5. But so does babel, tracuer, et al. Is there something specific you are referring to?

I remember it didn't use polyfills at all.

It does not, but again neither does babel, tracuer or other transpilers; so I'm not sure what you are driving at? We would have to use core-js polyfills either way (babel or TS).

I think the discussion is babel to transform ES6 -> ES5 or TypeScript to transform TS -> ES5. We have to go ES5 either way. TS can target ES6, and we could ship an ES6 build if we wanted to, but it would be in addition to the ES5 build.

[bigtimebuddy]

@photonstorm With TypeScript, to the best of my knowledge, you cannot pick and choose features to transpile to ES5 like you can with Babel.

I use TypeScript and think it's awesome. Would love to see Pixi adopt eventually. Google is now encouraging developers to use TypeScript with Angular, which is a good sign of wide-spread adoption. For a codebase as complex was Pixi, would be well served by strict-typings.

Some issues that would need to be addressed with TypeScript would include jsdocs (anyone have experience with this?) and upstream-dependencies who might be using the src when require('pixi.js') (is anyone requiring like this?).

As a first step, moving to ES6 is still a good option, since we'll need to convert to classes anyways. We could consider TypeScript another "pass" on modernizing the codebase once we're certain all concerns can be addressed.

[englercj]

With TypeScript, to the best of my knowledge, you cannot pick and choose features to transpile to ES5 like you can with Babel.

I had no idea you could do that with babel. I'm not sure I see the benefit of it for us since we have to target ES5 anyway. But that is neat!

Some issues that would need to be addressed with TypeScript would include jsdocs

https://github.com/TypeStrong/typedoc

upstream-dependencies who might be using the src when require('pixi.js') (is anyone requiring like this?).

We can point "main" to anything we want, and with typescript you point it to the built files (not the bundle). For example, TS files go into src/ and you transpile each file to js/ or something, then point main there. Then we also bundle it all up and put the bundles in dist/ or w/e. The npm package ships with the js/tsd files but not the TS source usually (debatable).

As a first step, moving to ES6 is still a good option, since we'll need to convert to classes anyways. We could consider TypeScript another "pass" on modernizing the codebase once we're certain all concerns can be addressed.

👍

[endel]

With TypeScript, to the best of my knowledge, you cannot pick and choose features to transpile to ES5 like you can with Babel.

They added this feature on TypeScript 2.0: https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#including-built-in-type-declarations-with---lib

It's usual to use ES5 as a target but include ES6 as libraries, to have the type definitions for things like Symbol, Map, etc.

Indeed, as @englercj said, you need to include shims after your code is compiled no matter which compiler you're using. Babel doesn't include a Symbol polyfill for IE9 automatically, for example.

[bigtimebuddy]

I had no idea you could do that with babel. I'm not sure I see the benefit of it for us since we have to target ES5 anyway. But that is neat!

For Pixi, not that useful, but yes, you can cherry pick Babel presets to select only certain features to transpile. This can be useful if you want to build to ES6 but only want to support bleeding-edge V8 features in Node 6, Electron 1, etc.

[photonstorm]

It's an interesting paradox really. Use ES6 to build with, because it's clean, and lovely, and very well supported / internally optimised by modern browsers. Yet have all that hard work destroyed by transpiling like it's 1995 :) (and with language syntax changes you can't shim around this).

I appreciate the problem is universal, nothing to do with TS or Pixi. Just a bit of a weird situation to be in.

[englercj]

@photonstorm It is an unfortunate irony 😞 hopefully we can have ES6 and ES5 builds so that for packaged apps (like electron) they can use the ES6 build.

[emilekberg]

Just jumping in to the discussion here :) I've seen people transpile TS to ES6 then use Babel to transpile it to ES5, i guess if you'd want something like certain features transpiled that would be quite nice?

Also, there is (yet another...) module bundler around, however i think this one seems to produce some pretty slick code, with the possibility of multiple outputs.
http://rollupjs.org/ it bundles from ES6/ES2015 module syntax, which i guess is pretty nice if you want to future proof the code.

i'm +1 for PIXI being written in Typescript. From my experience, type really does help a lot with the structure for projects like this. If only it can be kept performant :)

[photonstorm]

RollUp is fantastic, I love using it. Its tree shaking is seriously smart. Used with bubel (instead of babel) it makes for a really, really fast workflow.

Interesting to see so much TS love here. Certainly wasn't the case a year ago :) There are ways to type check ES2015 which may be worth considering.

[ivanpopelyshev]

@photonstorm didnt find bubel, but there's "buble"

[photonstorm]

Yeah yeah, typo. http://buble.surge.sh/

BubleTape is a nice test harness for it https://github.com/snuggs/buble-tape

[staff0rd]

Is #2936 the reason why Members dropped off the docs? This has one exposed member, where this has 25+.

Does @memberof need to be added to them now, or is there some magic that can be applied?

[englercj]

@staff0rd I think we are still cleaning things up, once it stabilizes a bit we will look into cleaning up the docs. Most likely it is just because the jsdoc version we are using has ES6 bugs. We should be able to get to cleaning this up soon.

[englercj]

ES6 has been merged to dev, thanks everyone who helped out. It is much appreciated!

Closing this for now.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.