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

Use ES6 modules #425

Closed
wants to merge 1 commit into from
Closed

Use ES6 modules #425

wants to merge 1 commit into from

Conversation

adroitwhiz
Copy link
Contributor

@adroitwhiz adroitwhiz commented Dec 9, 2019

Resolves #374
Resolves #417

This is a pretty big one--it's definitely a breaking change, so maybe hold off on merging until 0.7.0 gets a stable release and this can become part of 0.8.0.

This migrates the entire codebase over to ES6 modules, combined together using Rollup.

It builds on my work removing Underscore functions from the codebase. If you want, I can squash all those commits together, but I've kept them apart for ease of reviewing.

Because the changes involve removing a "wrapper" function from every file, everything has been decreased by one indentation level. To make the diff readable, you'll probably want to disable whitespace in the diff view.

The biggest change by far is that Two.Utils is gone. Every utility function previously present in Two.Utils has been moved to various modules in the src/utils folder. Because of this, they are no longer publicly accessible, which is the largest breaking change.

The sole exception is Two.Utils.Events, which I have chosen to leave under the Two.Utils namespace to avoid confusion with Two.Events, the list of event types. For a future version, I think it's a good idea to rename Two.Events to something like Two.EventTypes (which is what I named it internally), and then change Two.Utils.Events to be accessible as Two.Events.

utils/build.js should function exactly the same as before. I have changed Closure Compiler out for Terser because it's much faster; the minified code also seems slightly smaller but this could just be down to bundling differences. I have also added a build script to package.json which calls utils/build.js.

I've exposed Collection as Two.Collection; I'm not sure if this is desired. Should I keep it in the Two.Utils namespace?

ZUI is now included as part of the bundle; it currently depends on some private utility functions that it needs to pull in. The choice is between making ZUI part of the main two.js bundle, or packaging it separately and duplicating the dependencies.

I have not yet updated the JSDoc comments to indicate that the new classes are members of the Two.js namespace. I'll eventually address that in an additional commit.

@jonobr1
Copy link
Owner

jonobr1 commented Dec 9, 2019

😳😳😳

This is amazing!!! But, it's also going to be a second before I can address everything. I have finals the next two days.

Regarding Two.Utils.Events we can happily merge them. Two.Events can be the prototypical object with the event functions and a types property that has all the event names can be amended. This way all the information is encapsulated in the same reference.

One thing we can discuss here: I had been waiting for a full stable release of v0.7.0 in large part due to the new documentation branch you can see at jsdocs. It has a new folder and a few new npm run commands based on VuePress. It'll eventually replace the current gh-pages branch and be a repository more consistent with contemporary JavaScript practices on GitHub. As you've invested some considerable time into the project, I'd love any feedback you have on that branch especially regarding your latest changes here.

Thanks and will be addressing the overall commit soon!

@adroitwhiz
Copy link
Contributor Author

adroitwhiz commented Dec 9, 2019

It's great to see the site getting updated!

It's definitely a lot tidier than the current site, and the documentation being moved to a separate page helps with that. It's also really nice that everything's centered, as opposed to taking up the left half of the screen like the old site.

My opinion is that it looks a bit plain compared to the current site, however. I really like the color scheme and aesthetic that the current site has, and I'd like to see it carried over. Here's a quick mockup I made:

image

Maybe the big Two.js logo could even have some interactive element to it? Opening with such a demo would be a good way to show off what Two.js does.

@jonobr1 jonobr1 mentioned this pull request Dec 9, 2019
@jonobr1
Copy link
Owner

jonobr1 commented Dec 9, 2019

That's great to hear! Yes, the design of this new version hasn't been implemented yet... it's going to have an aesthetic inspired by the current site, but with more animations :)

@jonobr1
Copy link
Owner

jonobr1 commented Jan 29, 2020

Going to be merging this PR (link), so gonna close this one out because it's a duplicate. Thanks!

@jonobr1 jonobr1 closed this Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Two.Utils private? Switch to Import Statements
2 participants