BoxGeometry: Introduce ES6 classes and ES6-ES5 conversion for builds.

[Mugen87]

All IIFEs are gone, so we can actually introduce ES6 classes in the code base. I've started with BoxGeometry as a first step.

In order to ensure backwards compatibility, the PR introduces the usage of Bublé to perform a ES6 to ES5 conversion for three.js/three.min.js. So users of these build files should not notice any difference.

However, it is worth noting that Bublé will add an additional closure during the conversion process iff inheritance is used. The final code for BoxGeometry looks like so:

var BoxGeometry = /*@__PURE__*/(function (Geometry) {
	function BoxGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) {

		Geometry.call(this);

		this.type = 'BoxGeometry';

		this.parameters = {
			width: width,
			height: height,
			depth: depth,
			widthSegments: widthSegments,
			heightSegments: heightSegments,
			depthSegments: depthSegments
		};

		this.fromBufferGeometry( new BoxBufferGeometry( width, height, depth, widthSegments, heightSegments, depthSegments ) );
		this.mergeVertices();

	}

	if ( Geometry ) BoxGeometry.__proto__ = Geometry;
	BoxGeometry.prototype = Object.create( Geometry && Geometry.prototype );
	BoxGeometry.prototype.constructor = BoxGeometry;

	return BoxGeometry;
}(Geometry));

In any event, this approach enables the project to shift step by step to ES6 classes. In other words, it's not necessary to convert all files at once. It's possible to do this in batches (similar to the example modules) and verify each change for itself.

If this PR is going to be approved, I can take care about the others core files in the next time.

[Mugen87]

I've also verified the performance difference of ES6 classes vs. a pure prototype style. Unlike articles from 2015/2016, it seems classes offer now an equal performance.

http://jsbench.github.io/#b5d3ec32e4fae1606cafae8f309659aa

[EliasHasle]

👍 for going ahead with modernizing the code!

I don't understand the purpose of the added closure. It looks like the exact same outcome can be achieved without it, and without polluting the global namespace or anything.

This line puzzles me:

BoxGeometry.prototype = Object.create( Geometry && Geometry.prototype );

I did some testing with && on objects:

$ a = {a:"1", b:"2", c:"3"}
   {a: "1", b: "2", c: "3"}
$ d = {b:"2", c:"4", d:"5"}
   {b: "2", c: "4", d: "5"}
$ a&&d
   {b: "2", c: "4", d: "5"}
$ d&&a
   {a: "1", b: "2", c: "3"}
$ a && null
   null
$ null && a
   null
$ a && undefined
   undefined
$ undefined && a
   undefined

If the above case is analogous, the first operand to &&, which is Geometry, will be ignored, unless it is null or undefined, whereupon it will make the argument to Object.create null or undefined, respectively.

Is ES6-ES5 conversion not possible with Babel, rather than Bublé? If it is, will the output code look different?

[gkjohnson]

@EliasHasle That code is transpiled by Bublé and shouldn't be intended to be looked at or edited by anyone I don't believe. Regarding using && with objects -- Geometry && Geometry.prototype just ensures that the first object exists before evaluating the second part so an error is not thrown in the case that Geometry is null.

[looeee]

Is ES6-ES5 conversion not possible with Babel, rather than Bublé?

We could use Babel too, but Bublé does a much faster and simpler conversion. Although it's more limited than babel it should result in smaller file sizes since it doesn't add any helper functions to the code. As long as we're sticking with limited es6 features like classes it's probably better suited for use in a library like three.js.

On the other hand babel is much more configurable so I'm sure it's possible to avoid adding helper functions there too and output a similar file size to Bublé. If you want to do some testing I'd be interested in seeing the results. But my gut feeling is that we'll end up deciding that Bublé is the best choice anyway.

@Mugen87 perhaps you've already done some research here... What's your reason for choosing Bublé over Babel?

[Mugen87]

It was actually recommended by @mrdoob 😇.

I personally think if Bublé produces a sufficient outcome, I don't see the need for the more sophisticated Babel. I've tested some other ES6 features like default parameters with Bublé and everything works well.

[looeee]

I don't see the need for the more sophisticated Babel.

Yeah I agree. At least while we're just using a few easily transformed ES6 features, we don't need sophistication - and if we end up needing that later, or if someone demonstrates that Babel can be tweaked to output better or lighter code than Bublé, we can always switch.

With that said, the following is included for anyone who wants to test Babel's output:

Babel is now very simple to configure via @babel/preset-env and browser list.You just specify browsers that you want to target, and you can also tell it to skip adding polyfills and transforms with useBuiltIns:

{
  "targets": {
      "chrome": "50",
      "ie": "10",
      ...
   },
   "useBuiltIns": false
}

If we do later find that we want to switch to this setup, it comes with the advantage that we can officially state which version of each browser we support, and provide a simple configuration script that can be modified if people want to rebuild to support older browsers or other environments.

[Mugen87]

Sounds good!

[EliasHasle]

Will you eventually use/adapt https://github.com/Rich-Harris/convert-threejs-to-classes by @Rich-Harris, even though you closed #14654 in favor of this?

[Mugen87]

Nope, I've planned to do it file by file and verify each class manually. It's a good opportunity to review the code in general.

[mrdoob]

Lets do this!

[mrdoob]

Thanks!

[takahirox]

Don't we use let/const and arrow function yet?

[mrdoob]

I guess we can start using them?

[takahirox]

Yeah, I think so unless it has any problems.

[donmccurdy]

The block scope provided by let/const would have prevented a bug in code I've contributed, for one. 😇

Presumably we are only using ES6 features in src/* code? The same transpilation could be done when generating examples/js/* from examples/jsm/*, but users of the jsm/ version would lose support for old browsers.

[takahirox]

I guess block scope could also sometimes let JavaScript engine optimize the code better?

In addition to let/const and arrow function, it might be a good time to revisit some code styles, especially for loop. Perhaps for in/of, forEach, and others(? sorry I'm not familiar with ES6 standard) look more es6 standard styles. It may be good to switch to them if they're fast enough on modern browsers.

[mrdoob]

I would focus on just moving to classes for now. We can investigate these other changes later.

[mrdoob]

One step at a time.™ 😁

[takahirox]

I would focus on just moving to classes for now.

Agreed. Just in case, only in src/* so far?

[mrdoob]

Yep.

[EliasHasle]

Off-topic about block-scoped variables

The block scope provided by let/const would have prevented a bug in code I've contributed, for one.

for ( let i = 0; i < 10; i++ ) is almost always better than for ( var i = 0; i < 10; i++ ), because the latter will be function-scoped. One may just as well use as local scoping as possible to begin with, to avoid "unexplainable" bugs. Even if you want to access the final value of i, you can just do:

let i;
for ( i = 0; i < 10; i++ ) {
    doSomething();
}
console.log(i);

Also, the local scoping of the iteration counter lets you define callbacks in the loop using the value. Otherwise, you get multiple references to the function-scoped variable, and the traditional solution involves a horribly hard-to-grasp IIFE within the loop.

[Mugen87]

@EliasHasle Please let's discuss such stuff in other issues at the right time. Classes now, other ES6 features later.

[EliasHasle]

OK. :-)

[andrevenancio]

I had a crappy week so far at work, but seeing this moving forward is making up for it! <3 well done

[mrdoob]

@Mugen87

However, it is worth noting that Bublé will add an additional closure during the conversion process iff inheritance is used.

Any chance you could investigate and see if we can get rid of that additional closure? I looked at Bublé options but I didn't figure it out. Maybe ask on Bublé`s repo?

[Mugen87]

Done 👍: bublejs/buble#219

[mrdoob]

Thanks!

[Rich-Harris]

Ah, I'd started writing a reply yesterday but got sidetracked — sorry. It went something like this:

Maybe ask on Bublé`s repo?

Unfortunately Bublé isn't really maintained any more. We built it for an era when ES2015 was still the future, but other aspects of modern JavaScript that it doesn't accommodate (primarily async/await) weren't yet widely used. Nowadays, all browsers support all of ES2015 (and way beyond), rendering Bublé and its ilk unnecessary for most cases.

Essentially, the only browser that doesn't support class natively (with usage that isn't a rounding error, at least) is IE11. Which means that if you transpile ES2015 down to ES5, you're penalising (with a larger build) the 98% for the sake of the 2%.

I'd advocate for leaving class untranspiled, and maybe having a separate three.legacy.js build that is transpiled, where it doesn't matter so much about the extra bytes.

[mrdoob]

Unfortunately Bublé isn't really maintained any more.

Oh, that's unfortunate...

I'd advocate for leaving class untranspiled, and maybe having a separate three.legacy.js build that is transpiled, where it doesn't matter so much about the extra bytes.

That's kind of the plan. three.module.js is ES6 with modules/classes. But we're trying to produce the same three.js and three.min.js we currently have.

[EliasHasle]

Moved to #15176.

[donmccurdy]

EDIT: Moved comment to #15176 (comment).

[Mugen87]

Should there also be a three.module.min.js, then, to serve as the new standard save-bandwidth build?

Please discuss this issue here: #15176

[looeee]

Unfortunately Bublé isn't really maintained any more.

@Rich-Harris that seems to be the case with butternut as well. Do you think it would be a good idea to leave a note at the top of the readme for each repo warning people of this? It could save a lot of wasted dev hours.

[Rich-Harris]

@looeee good idea — I've archived Butternut and opened an issue on Bublé to discuss (in case any of the other people who have contributed to it in the past intend to do so again in the future)

[mourner]

Unfortunately Bublé isn't really maintained any more.

@Rich-Harris I'd say it's probably a little premature to call the project unmaintained given it got 2 releases and several dozen commits in 2019 alone, and still powers software in production used by millions of people (such as Mapbox GL JS).

I would wholeheartedly support deprecating any kind of transpilation in favor of shipping optimized ES module bundles if this were up to me alone; however, the truth is most of the enterprise world is stuck supporting IE11 for an undefined amount of time, it's a hard requirement from most of our big clients, so I will continue making sure Buble works for our needs in the nearest year at least.

[Rich-Harris]

@mourner that's fair, I'll walk that back a bit. I meant more in the sense that it's not generally gaining new features, and I think it's only fair that projects be aware of that before adopting it. Very grateful for your commitment to the project!

I guess my real point was that if we encourage library users to use modern ESM, it's not the worst thing in the world if people actively choosing to support IE11 need to pay a 1 or 2% transpilation tax.

[angelinaqueen]

See

[mrdoob]

#19934