-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
8000053
commit 501053e
Showing
5 changed files
with
117 additions
and
128 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,16 @@ | ||
import { Object3D } from '../core/Object3D.js'; | ||
|
||
function Group() { | ||
class Group extends Object3D { | ||
|
||
Object3D.call( this ); | ||
constructor() { | ||
|
||
this.type = 'Group'; | ||
super(); | ||
this.type = 'Group'; | ||
Object.defineProperty( this, 'isGroup', { value: true } ); | ||
|
||
} | ||
|
||
Group.prototype = Object.assign( Object.create( Object3D.prototype ), { | ||
|
||
constructor: Group, | ||
} | ||
|
||
isGroup: true | ||
|
||
} ); | ||
} | ||
|
||
|
||
export { Group }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,18 @@ | ||
import { Line } from './Line.js'; | ||
|
||
function LineLoop( geometry, material ) { | ||
class LineLoop extends Line { | ||
|
||
Line.call( this, geometry, material ); | ||
constructor( geometry, material ) { | ||
|
||
this.type = 'LineLoop'; | ||
super( geometry, material ); | ||
this.type = 'LineLoop'; | ||
Object.defineProperty( this, 'isLineLoop', { value: true } ); | ||
|
||
} | ||
|
||
LineLoop.prototype = Object.assign( Object.create( Line.prototype ), { | ||
} | ||
|
||
constructor: LineLoop, | ||
|
||
isLineLoop: true, | ||
|
||
} ); | ||
} | ||
|
||
|
||
export { LineLoop }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is breaking my inheritance:
Is there a way to make it work without ES6 syntax?
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes#sub_classing_with_extends
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrdoob I've added a note in the migration guide that
Object3D
as well as other basic classes or now written in ES6 class syntax.https://github.com/mrdoob/three.js/wiki/Migration-Guide#127--128
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is for 126 => 127 I think
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
THREE.Group
was already converted inr127
. I guess more devs will complain in context ofObject3D
andShaderMaterial
.However, it does not make sense to list all converted classes in the migration guide. The upcoming release will contain the biggest (and final) step towards ES6 classes.
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mugen87 @marcofugaro Maybe we could do a new build
three.legacy.js
that transpiles to ES5?501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FlorentMasson How do you structure your projects? Are you using the
npm
package ofthree.js
and already use ES6 import syntax with a build? Or do you copy library files into your project and import them via the script tag?501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrdoob It would be indeed convenient for users to have
three.legacy.js
. However they could requests legacy files forexamples/js
, too. And it won't help users wit a npm/ES6 import workflow since these files would still be ES6.Right now, I would vote to leave the ES5 conversion to users.
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @Mugen87, apart from a couple of users, ES6 is pretty widespread nowadays and will continue to be.
I guess we could write a guide with this comment's content to point the users to.
501053e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I use three.js through npm.
I do not use ES6 modules but I use ES6 syntax when appropriate (hence @mrdoob extract is actually not problematic).
My project is a set of javascript files (own source and third-party code) included via script tags (e.g. pointing node_modules/... when appropriate)
I remember trying ES6 import syntax for three.js but you can't import an ES6 module into a non-ES6 module.
For release, I have a script that concatenates all javascript files, add a closure and processes that through UglifyES for minification and obfuscation, and with option
ecma:5
for best compatibility (people sometimes use older browsers/version especially on mobile).That's why using ES6 syntax is ok for us, but not being forced to use ES6 modules (until someone knows about a way to import ES6 modules outside a module)