Evaluate ES6 classes

[arctwelve]

Why is this 'idiom' of inheritance being introduced into the code:

PointLight.prototype = Object.assign( Object.create( Light.prototype ), {

Seriously? Function(nested Function(ParentPrototype) COMMA SHOTGUN BRACKET?

The faithful two line style still in, for example, the Materials classes are much clearer and cleaner. Assign the prototype and then set the constructor. The end. Please don't ruin the library by catching JavaScript disease - the bizarre need to masturbate the way objects and inheritance are coded. One style throughout the library. No need to change it.

[mrdoob]

So you're suggesting using this pattern instead?

PointLight.prototype = Object.create( Light.prototype );
Object.assign( PointLight.prototype, {

[sasha240100]

class PointLight extends Light

hehe 😄 And no problems...

[looeee]

@sasha240100 someday...

[looeee]

@mrdoob not quite - the two ways you mention are directly equivalent. I think the OP is comparing

PointLight.prototype = Object.assign( Object.create( Light.prototype ), { 
    constructor: PointLight,
    prop1: 'something',
    method1: function someFunction() { .. },
    ...
});

with

function PointLight () { ... };

PointLight.prototype = Object.create( Light.prototype );

PointLight.prototype.constructor = PointLight;

PointLight.prototype.prop1 = 'something';

PointLight.prototype.method1 = function someFunction() { .. };

...

Which is the way it's done here for example.
As far as I can see these styles are equivalent - is there something I'm missing?
Or was the style changed to use Object.Assign once it became available and not updated across the codebase?

[sasha240100]

@looeee @bfred-it @mrdoob Why not just using rollup-babel ?

Comparison:
Current. es5 + es6 harmony modules.

import { LineBasicMaterial } from './LineBasicMaterial';
import { Color } from '../math/Color';

function LineDashedMaterial( parameters ) {

	LineBasicMaterial.call( this );

	this.type = 'LineDashedMaterial';

	this.scale = 1;
	this.dashSize = 3;
	this.gapSize = 1;

	this.setValues( parameters );

}

LineDashedMaterial.prototype = Object.create( LineBasicMaterial.prototype );
LineDashedMaterial.prototype.constructor = LineDashedMaterial;

LineDashedMaterial.prototype.isLineDashedMaterial = true;

LineDashedMaterial.prototype.copy = function ( source ) {

	LineBasicMaterial.prototype.copy.call( this, source );

	this.scale = source.scale;
	this.dashSize = source.dashSize;
	this.gapSize = source.gapSize;

	return this;

};


export { LineDashedMaterial };

ES2015+. Same code, but es2015+ with babel-plugin-transform-class-properties:

import { LineBasicMaterial } from './LineBasicMaterial';
import { Color } from '../math/Color';

export class LineDashedMaterial extends LineBasicMaterial {
  type = 'LineDashedMaterial';

  scale = 1;
  dashSize = 3;
  gapSize = 1;
  isLineDashedMaterial = true;

  constructor(parameters) {
    super();
    this.setValues( parameters );
  }

  copy(source) {
    super.copy(source);
    
    this.scale = source.scale;
    this.dashSize = source.dashSize;
    this.gapSize = source.gapSize;

    return this;
  }
}

ES6 features that would simplify three.js code:

• Default parameter values.
  • The can easily replace such parts of code
• Property shorthand
  • Can be used here
• Arrow function
  • https://github.com/mrdoob/three.js/blob/dev/src/loaders/JSONLoader.js#L36

[mrdoob]

I'm all for moving to ES2015+, we just need to find a way to output similar code than what we currently have out of it, so the performance stays the same in all cases.

[Mugen87]

I have a question in context of classes. How would we transfer methods like Vector3.unproject into the class syntax? The method actually uses a closure in order to create a new scope. This is an important mechanism that keeps the amount of object creations as low as possible.

Do we need Object.assign in these cases?

[sasha240100]

@Mugen87 @mrdoob Some interesting info on es6 performance. Especially on Object.assign:

From this article

[satori99]

How would we transfer methods like Vector3.unproject into the class syntax? The method actually uses a closure in order to create a new scope.

@Mugen87 Can't they just be non-exported module scoped objects? Something like this;

const tempMatrix = new Matrix();    

export default class Vector3{
    unproject() {
        // uses tempMatrix
    }
}

[Mugen87]

Ah yes, i think this should work 😊

[mrdoob]

Last month this was suggested on twitter:

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

[sasha240100]

@mrdoob Wow! It seems already working. Can't we make a branch, transform some classes to es6 and see how it compiles?

---

@satori99 As an idea of how to keep tempMatrix inside Vector3 code to avoid problems with globals:

export default class Vector3 {
    static tempMatrix = new Matrix();

    unproject() {
        // uses Vector3.tempMatrix
    }
}

[mrdoob]

@mrdoob Wow! It seems already working. Can't we make a branch, transform some classes to es6 and see how it compiles?

Sound good to me! I'm currently focusing on WebVR so it'll need to be someone else than me.

[satori99]

@sasha240100 The benefit of using module scoped vars is that they remain hidden from regular user code, which seems appropriate for temp variables.

I don't mind the "We are all adults here" pythonistic approach in regards to private vars, but temp variables shouldn't really pollute the namespace unnecessarily.

Also, It would be nice if those of us who have enabled native module support in our browsers could load the src files directly. This is a much more pleasant way to develop, without needing watchers and transpiling after every edit. Using class properties means this isn't possible as they are not part of the current class spec.

[agnivade]

Apologies for butting in. We should probably change the issue title to something like - "Evaluate ES6 classes", since now the thread has changed to something completely different.

[pailhead]

Why change the pattern @Mugen87 @satori99 ?

method =(()=>{ 
    const vec3forThisScope =...; 
    return (arg)=>{...}
})()

[FishOrBear]

Why not try typescript, it can be compiled into other versions of js

[joejordanbrown]

TypeScript really would be a great option to think about because it's a transpiler + type checker and a superset of JavaScript, so it's easy to move a code base over to .ts files and gradually refactor to ES6 with type checking.

It may sound scary if you've never used TypeScript, but it's really not a big learning curve and would be a small price to pay for the benefits it would bring. The TypeScript community would be very happy to help with this transition and creating performance testing against the current library to make sure it's not being downgraded.

A few useful articles:

• Why You Shouldn't Be Scared of TypeScript
• TypeScript vs ES6 vs ES2015
• TypeScript won
• What is TypeScript and why with Anders Hejlsberg

To quote of core developer Anders Hejlsberg, TypeScript was born in response to complaints from clients and internal teams that JavaScript didn't lend itself well to large applications.

The goal was to "strengthen JavaScript with things like classes, modules, and static typing", without sacrificing the advantage of its being open-standards and cross-platform; the result was a "language for application scale javascript development", built as a superset of the language.

[mrdoob]

Until browsers can execute natively TypeScript, I prefer to keep using JavaScript.

[joejordanbrown]

@mrdoob

I can't see that as a valid reason for not using TypeScript solely on the reason it can't be run directly in the browser. You wouldn't want it to run in the browser because of all the extra lines of code that is intended only for compile time checking. It's not currently a runtime checking language. So if it was ever used in the browser it's more than likely going to have all the typed code stripped away because it impacts performance and this would then be vanilla JavaScript code.

I think you're totally missing the point of using a typed language and the benefits it has in development in a large code base. You are still writing and using JavaScript, the whole point of TypeScript is that it is a superset of JavaScript. You write JavaScript with types, that's compiled into JavaScript in the specified ECMAScript target version, which is configurable in the compiler options, the permitted values are 'es3', 'es5', 'es2015', 'es2016', 'es2017' or 'esnext'.

Because Typescript is JavaScript it makes it possible to progressively migrate without having a massive headache of refactoring everything at once. It can be gradually done and improved by the community. It's no more work than what's being discussed here with refactoring to use ES6 classes. That's the only reason I mention it here instead of opening a new issue.

See TypeScript playground links below for great examples:

• Classic JavaScript Example
• Adding Types Example
• Adding Types with error Example
• Using Classes Example
• Using Classes with error Example

[pailhead]

@joejordanbrown

Can you think of anyone that could possibly disagree with you about typescript being the best solution to this particular problem?

If you type in typescript vs into google, a few terms pop up, one of them being Flow. Searching for that, seems to yield a number of articles where people are debating the pros and cons of these two.

No types seems like more of a compromise than choosing one of these.

[arctwelve]

Save Typescript for projects that are more complicated than the result they create -- specially frontend frameworks that could have been implemented in HTML in the first place. My original point was to get rid of the JavaScript disease, not make it worse. JavaScript is a simple almost toy language that sometimes is used for complex results like three.js. Typescript is pointless.

…

On Sep 6, 2017, at 1:55 PM, Joe ***@***.***> wrote: @mrdoob I can't see that as a valid reason for not using TypeScript solely on the reason it can't be run directly in the browser. You wouldn't want it to run in the browser because of all the extra lines of code that is intended only for compile time checking. It's not currently a runtime checking language. So if it was ever used in the browser it's more than likely going to have all the typed code stripped away because it impacts performance and this would then be vanilla JavaScript code. I think you're totally missing the point of using a typed language and the benefits it has in development in a large code base. You are still writing and using JavaScript, the whole point of TypeScript is that it is a superset of JavaScript. You write JavaScript with types, that's compiled into JavaScript in the specified ECMAScript target version, which is configurable in the compiler options, the permitted values are 'es3', 'es5', 'es2015', 'es2016', 'es2017' or 'esnext'. Because Typescript is JavaScript it makes it possible to progressively migrate without having a massive headache of refactoring everything at once. It can be gradually done and improved by the community. It's no more work than what's being discussed here with refactoring to use ES6 classes. That's the only reason I mention it here instead of opening a new issue. See TypeScript playground links for examples: Classic JavaScript Example Adding Types Example Adding Types with error Example Using Classes Example Using Classes with error Example — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[pailhead]

^ i'd be fine even with the monstrous pattern, as long as it's consistent.

[mrdoob]

@joejordanbrown sounds like you're in love with typescript. feel free to fork the project and port it to typescript. three.ts! 🙌

[joejordanbrown]

@pailhead

That's a matter of choice, I'm sure many will agree and disagree, that's normal though right! You're always going to see "this vs that", "mine is better than yours". I understand that each has their own benefits. It's about weighing up the options available and seeing if they can benefit the project. Comparisons are a good thing, it pushes projects further.

You mention Flow, the problems I see with that are:

• Flow licensing is BSD 3-clause "Facebook BSD+Patents License", Apache Software Foundation banned the use of this license in new projects. You can read more details here.

• IDE support is lacking compared to TypeScript.

• User base is small compared to TypeScript,

• Available typings for public libraries are incomplete, TypeScript has plenty of well-maintained typings.

• Documentation and resources are hard to find and are vague compared to TypeScript you will find great documentation, books, videos and lots of other e-learning resources.

• Flow uses .js files that are flagged with // @flow, this can be confusing because you see the .js extension so expect JavaScript but in fact, it's FlowType. Where as TypeScript uses its own extension .ts. This also allows you to have the TypeScript and JavaScript output files with identical names in the same directory, which is ideal for a small project, obviously, that wouldn't be the case on a large project though because you'd be using a build system to manage the build process.

Even google is backing TypeScript in a big way, that shows the confidence they have in TypeScript. Read the post here or here.

Typescript has become allowed for unrestricted client development as of March 2017. TypeScript and Angular on TypeScript are used in Google Analytics, Firebase, and Google Cloud Platform and critical internal tools such as bug tracking, employee reviews, and product approval and launch tools.

I just thought I'd open the discussion about using a Typed language and see what everyone else thinks about the idea. It does seem that @mrdoob is totally against even discussing the idea though.

---

@arctwelve
I don't see how this project isn't complicated and how using a Typed language would affect it negatively.

---

@mrdoob
Not at all, I can just see the benefits it could have especially if a new branch is being created to update to ES6 classes. I think to answer with create your own fork called three.ts is just being silly. That's really against good OSS practices if everyone just forked OSS projects and modified their own source code instead of focusing on the 1 project and making it the best it can be. You would end up with really poor software or communities splitting and focusing on the project they preferer for whatever reason. Why can't you have an open discussion about the pros and cons?

[pailhead]

Not to play the devils advocate, but it seems like he did

have an open discussion

it was just really short :)

I share a similar standpoint, it's a JS library and JS is standardized. You can't go wrong with choosing JS for a JS library, while you can if you choose something else. I just took Flow as one of the alternatives to typescript, dunno if there are others.

Anyway, it seems like we really went off topic.

Mugen87 changed the title from
Remove JavaScript Disease to Evaluate ES6 classes

The original title referred to (as far as i understand) to the lack of style consistency. In particular using Object.assign() in some places, and another pattern in others.
If i'd evaluate anything here, it would be the current title of the issue. Why is the issue of consistency being elevated to a discussion about using a new language?

I imagine that with both typescript and es6, the code should be fairly consistent.

I'd address this issue by updating this page:

https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2

and adding either:

A) "...use Object.assign ..."
B) "...dont use Object.assign"

[pailhead]

One style throughout the library. No need to change it.

The faithful two line style still in, for example, the Materials classes are much clearer and cleaner.

It's in the first post.

I suggest:

1. edit the title to reflect this sentence, discuss having one style throughout the library, editing the style guide etc.
2. start a new discussion titled "evaluate es6 classes", where es6 classes would be evaluated
3. start a new discussion titled "evaluate having three written in a typed language", where typescript and such would be discussed

[mrdoob]

Anyway, it seems like we really went off topic.

Indeed. @joejordanbrown feel free to create a new topic to discuss TypeScript.

Btw, it's also bad OSS practice to ignore previous conversations... #341 (comment)

[DefinitelyMaybe]

Reposted from closed issue.
Hi there,

I wanted to create this issue to try and tease out everyone's thoughts on how we'd like to move forward with class migration.

My quick summary of what I've come across so far:

• We should deprecate the examples folder before doing anything else
• There are parts of src/ that aren't extended by any of the examples that could be converted
• With some changes to the modularize.js script we could get started on examples/js/ which generates the corresponding examples/jsm scripts.

Have I missed anything?

[donmccurdy]

We should deprecate the examples folder before doing anything else

I'm not sure who is responsible for any specific next step on the examples/js folder. Unless we can articulate that plan, I'd prefer that this didn't block converting things to ES classes. For that reason, I somewhat disagree with #11552 (comment). 🙂

[DefinitelyMaybe]

further to that, it appears that we're just waiting on a date to be set

[DefinitelyMaybe]

Also, as part of a bit of revision I did, I found this comment about how other ES2015 features could be introduced via the current rollup build process. There is even an example given.

edit: here's a gist of a another quick example

[DefinitelyMaybe]

Let's start the conversion train.
File dependencies comment.

[ianpurvis]

@DefinitelyMaybe I've got some time and I'd love to help. Can I take some items on the dependencies.json list?

[DefinitelyMaybe]

👍

let's call dibs on things

[ianpurvis]

@DefinitelyMaybe What is the best way to deal with inheritance from a deep ancestor like Object3D? For example, I ran into breakage converting src/audio/AudioListener.js:

[ROLLUP] bundles src/Three.js → build/three.js...
[ROLLUP] (!) Error when using sourcemap for reporting an error: Can't resolve original location of error.
[ROLLUP] src/audio/AudioListener.js: (137:1)
[ROLLUP] [!] Error: 'return' outside of function
[ROLLUP] src/audio/AudioListener.js (137:1)
[ROLLUP] 135:   };
[ROLLUP] 136: 
[ROLLUP] 137:   return AudioListener;
[ROLLUP]        ^
[ROLLUP] 138: }(Object3D));
[ROLLUP] Error: 'return' outside of function

[DefinitelyMaybe]

we were going to make a note at the top of the file about any issues we run into, if Im remembering correctly.

[ianpurvis]

I misunderstood what was happening with AudioListener... After some debugging, I think there is a matching issue in the bubleCleanup transform. See #19934 (comment) for some more info. I ran into this with a few different classes, so we'll probably have to get it sorted before going much further.

[DefinitelyMaybe]

this will be the case for some of the files but not all. We were anticipating such an issue.

[DefinitelyMaybe]

For those following along, please have a quick read of the discussion over here.

In the mean time, dibs on src/loaders!

[Mugen87]

Closing in favor of #19986.