Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES6ify Two.js Codebase #592

Merged
merged 157 commits into from
Jan 9, 2022
Merged

ES6ify Two.js Codebase #592

merged 157 commits into from
Jan 9, 2022

Conversation

jonobr1
Copy link
Owner

@jonobr1 jonobr1 commented Jan 6, 2022

🎯 Affects issues:

  1. Use ES6 features in the codebase? #509
  2. [Bug] Cannot import two.js in typescript project #531
  3. [Enhancement] Add return type for vector functions #542

📋 This is a more-or-less entire rewrite of Two.js to use EcmaScript 6 features. In particular:

  • const / let instead of var for tighter scoping
  • class construction instead of prototypical modifications
  • Expanded classification of classes. For instance:
    • Two.Events is now a class
    • Two.Element is a new base class of Two.Shape, Two.Gradient, and anything else that can be queried in the scenegraph
  • Where possible functions are named instead of anonymous
  • Removes all MakeObservable methods in favor of Object.defineProperty invocations on constructor
  • Module imports are through typical exports and except for the root Two.js class, not with default. So you'll need to import specific modules like so:
import { Vector } from 'two.js/src/vector.js';
var v = new Vector();

🏁 These changes allow for improved:

  • TypeScript Declarations (fully expanded and invoked through TypeScript's types compiler)
  • Improved documentation
  • Code legibility and OOP style
  • Performance debugging
    • Easier to identify culprit functions in Chrome et al. performance debug console

⚠️ These changes break:

  • Loose interoperability between Two.Vector and Two.Anchor. For any curve, it's required you use anchors instead of vectors now.

🗒️ All tests and first party examples are passing

@lgtm-com
Copy link

lgtm-com bot commented Jan 6, 2022

This pull request introduces 9 alerts when merging 891d132 into fc7f667 - view on LGTM.com

new alerts:

  • 3 for Superfluous trailing arguments
  • 1 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Variable not declared before use
  • 1 for Wrong use of 'this' for static method
  • 1 for Unused variable, import, function or class
  • 1 for Access to let-bound variable in temporal dead zone

@jonobr1 jonobr1 self-assigned this Jan 6, 2022
@jonobr1 jonobr1 added this to the v0.8.0 milestone Jan 6, 2022
@lgtm-com
Copy link

lgtm-com bot commented Jan 6, 2022

This pull request introduces 1 alert when merging 6a16043 into fc7f667 - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@jonobr1 jonobr1 merged commit 3bc5d38 into dev Jan 9, 2022
@jonobr1 jonobr1 deleted the es6 branch January 9, 2022 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant