Convert to ES2015 classes, for a smaller/faster bundle

[Rich-Harris]

Hi there! Please forgive the size of this PR; I'm opening it more to start a discussion than anything, and I chose this over opening an issue (or adding to one of the existing ones) because it felt like it would be beneficial to a) be able to show exactly what I'm talking about, and b) prove that it's actually possible.

tl;dr

We can make Three.js smaller and faster to load — and hopefully pave the way for future improvements like treeshakeability — by switching from functions to ES2015 classes. This PR was generated with convert-threejs-to-classes, which provides some more information.

Background

There have been a couple of issues in the past suggesting migrating to ES2015 classes — e.g. #11552 and #6419 — but at the time those issues were raised it was more of an encumbrance than anything, since browsers didn't yet support the class keyword.

Now, though, all browsers support classes, and performance is excellent. No transpilation is needed to use them. As of this PR, build/three.min.js is 5% smaller than the current build, and startup time is quicker.

Does everything work?

All the unit tests pass. Most of the examples still work, but a few need updating because switching to classes introduces a breaking change — you can no longer do this...

function MySubclass() {
  MySuperclass.call(this); // this call is illegal!
}

...but must instead do this:

class Mysubclass extends MySuperclass {}

Some of the examples may be tricky to update automatically; these could be addressed in a follow-up PR if it turns out to be prohibitively difficult.

Edit: As noted below, introducing class means dropping IE11 support. Devs who need to support IE11 can do so via e.g. Bublé.

Is it really worth it?

That's for the maintainers to decide. But I believe this would be the best decision for the project — classes are easier to reason about, easier to statically analyse (which raises the tantalising possibility of proper tree-shakeability!), and are the 'officially sanctioned' and modern way to write this sort of code.

The downside is the breaking change outlined above, but it's one that is very easily addressed in app codebases. May as well rip the bandaid off now!

Follow-up work

Aside from fixing the examples mentioned above, there are some further tweaks to the codebase that could be applied manually in a second PR — see details here.

---

Let me know if this is something you'd be interested in doing. Thank you! 🍻

[Rich-Harris]

(one note, as I try to fix the CI errors before I leave the office — the build-uglify script doesn't work in this branch because Uglify only supports ES5. We can easily fix it by replacing it with terser, a fork that supports modern JavaScript)

[mrdoob]

(one note, as I try to fix the CI errors before I leave the office — the build-uglify script doesn't work in this branch because Uglify only supports ES5. We can easily fix it by replacing it with terser, a fork that supports modern JavaScript)

I only use build-closure. Not sure who uses build-uglify 🤔

[Andarist]

What would be the official recommendations for people who have to support IE11 (which is far from being dead)?

[IvanSanchez]

What would be the official recommendations for people who have to support IE11 (which is far from being dead)?

Transpile your code so it looks like a UMD package. See e.g. https://github.com/Leaflet/Leaflet/pull/6021/files - the Leaflet codebase is ES2015 modules, but the UMD bundle supports down to IE8.

The rollup.config.js file here also creates a UMD bundle, so I don't foresee any major obstacles regarding browser support.

[Andarist]

Ok, sure - but how i.e. webpack users are supposed to consume those UMD bundles, should they chose:

import Lib from 'some_lib/dist/some_lib.umd.js'

over

import Lib from 'some_lib'

Seems like a big burden on users to do that for "each" library they use (assuming that shipping es6+ code to npm becomes a trend).

[arodic]

@Rich-Harris this is amazing work. Assuming this goes through, I can help fixing some of the examples.

@Andarist

Seems like a big burden on users to do that for "each" library they use

Perhaps this is a good thing! Lack of burden/friction is how we found ourselves in a situation that most trivial apps have hundreds of dependencies. Remember leftpad?

assuming that shipping es6+ code to npm becomes a trend.

I wholeheartedly hope ES6+ becomes a trend as soon as possible. However, I am not convinced that npm is the best place to ship it.

[IvanSanchez]

how i.e. webpack users are supposed to consume those UMD bundles

If a webpack user wants to consume the UMD bundle, then they must import the whole file. If a webpack user wants to consume the ES2015 exports, then Webpack shall take care of transpiling down to something that IE can understand.

Please note that this PR does not change the shape of the final bundles (build/three.js is still a UMD bundle, and build/three.module.js is still a ES2015 module bundle). So whatever issues you might have regarding webpack, they won't change a bit with this PR.

[Rich-Harris]

What would be the official recommendations for people who have to support IE11 (which is far from being dead)?

I actually misread the compat table (which has a bug — before it fully loads, the boxes in the table are offset by one column!), and thought IE11 would still be supported. My mistake.

That said, IE11 use is around 2.5%; among users of WebGL apps, I would expect the number to be much lower still. We shouldn't let a tiny minority of holdouts levy a tax on the rest of the web. For app developers that really do need to support IE11, it's easy enough to run the code through a transpiler; if you don't want the added bulk of a Babel transpilation then you could use Bublé, which generates a minified bundle almost identical in size to the current one. (We could even opt to include a pre-transpiled bundle in the package, though I'd argue it's unnecessary.)

Ok, sure - but how i.e. webpack users are supposed to consume those UMD bundles, should they chose:

The package.json has "main": "build/three.js" and "module": "three.module.js" — in other words, if you're consuming Three via webpack you're already consuming the bundle. This PR doesn't change anything around that.

I wholeheartedly hope ES6+ becomes a trend as soon as possible. However, I am not convinced that npm is the best place to ship it.

I have to disagree with that 😀 It's totally reasonable to ship ES2015 code in 2018, and not doing so is being unkind to our users' data plans and causing unnecessarily slow app startup times (since ES5 code tends to be significantly larger than ES2015+ code). For my own libraries, I usually draw the line at whether or not the features I'm using run in all current browsers (which excludes IE11) and Node 6 (the maintenance LTS version).

Thanks for the feedback everyone!

[mrdoob]

Thanks for working on this Rich!

I think I would focus on src/ first... It would be a no-brainer if, by using Babel, we were able to produce identical build/three.jsandbuild/three.min.js` or with minimal changes. We found a configuration last year that had pretty good output:

https://astexplorer.net/#/gist/9db8b707e3f69f94ded4af737f879fa7/b12fb12806e97df96278ec8b7d52018c8421872f

With that solved then we can figure out how to deal with examples/ in a different PR.

What do you think?

[looeee]

@Rich-Harris what's the reason for choosing Bublé over Babel here?

[Rich-Harris]

Thanks @mrdoob — I've updated the PR to only affect src files. I couldn't figure out how to integrate the custom Babel plugin (more specifically, it errored out on certain files when I ran it locally), so for now it uses Bublé, which has similar output (the Babel output I'm using for comparison is the one from AST Explorer):

• current dev branch: 549321 bytes minified, 135780 zipped
• classes: 519783, 132122
• classes + Babel: 550824, 135049
• classes + Bublé: 551321 / 133706

As you can see the Bublé output is very slightly larger than hand-written functions, or the output of the custom Babel plugin, but the gzipped size is actually slightly smaller. All the examples I've checked so far continue to work.

Of course, preserving the classes results in the smallest output, but it does entail that breaking change so I understand if you'd prefer to do that at a later point. If you'd prefer to use Babel I can try and get that working later.

[looeee]

Ha, you answered my question at the exact second I asked it 😆

[Rich-Harris]

@looeee we commented at the exact same time — see my previous comment for the rationale. Basically, I couldn't get Babel to work, and Bublé's output zips better. (Also, Bublé is much faster 😀 )

[Mugen87]

It seems some files are missing e.g. Box3 or Vector3. I wanted to check how the following type of methods are handled (where the actual implementation is wrapped in an IIFE). This style is used in order to avoid object creation:

three.js/src/math/Box3.js

Lines 328 to 342 in 4688a5f

	intersectsSphere: ( function () {
	var closestPoint = new Vector3();
	return function intersectsSphere( sphere ) {
	// Find the point on the AABB closest to the sphere center.
	this.clampPoint( sphere.center, closestPoint );
	// If that point is inside the sphere, the AABB and sphere intersect.
	return closestPoint.distanceToSquared( sphere.center ) <= ( sphere.radius * sphere.radius );
	};
	} )(),

AFAIK, you can't do the same with classes. Helper objects like closestPoint would need to be placed in module scope. Do we have to change this manually?

[Rich-Harris]

@Mugen87 Box3, Vector3 and Quaternion were excluded from the initial conversion because classes don't get hoisted in the same way as functions; because those classes are eagerly instantiated inside those IIFEs, we get an error when evaluating. So I left them as functions for now.

I added an example of how we could convert methods like intersectsSphere over here: https://github.com/Rich-Harris/convert-threejs-to-classes#follow-up-work. Basically yes, you're right, it will need to be a manual follow-up to the automated conversion. The good news is that removing those IIFEs should get rid of the tricky cyclical dependencies, speed up startup (if only by a tiny amount), and put us on the path towards a more tree-shakeable Three.js!

[Mugen87]

How about changing the respective methods first (so remove the IIFEs) and then use your tool for auto-conversion? In this way, the shift to classes might be maybe easier.

[Rich-Harris]

@Mugen87 good idea! done #14695

[mrdoob]

Some progress... #15280

[Mugen87]

Closing in favor of #17276