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

Replaced Google Closure compiler with Terser #20354

Merged
merged 6 commits into from
Sep 15, 2020
Merged

Replaced Google Closure compiler with Terser #20354

merged 6 commits into from
Sep 15, 2020

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Sep 15, 2020

Giving Terser a try, we can revert if we bump into problems.

This simplifies the build setup so it's all done inside rollup now.
The script also automatically adds the // threejs.org/license header so we won't have to worry about it anymore.

@mrdoob mrdoob added this to the r121 milestone Sep 15, 2020
@mrdoob

This comment has been minimized.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 15, 2020

Creating three.module.min.js is also straightforward.

Please consider #15176 (comment). I'm afraid the existence of three.module.min.js will cause a lot of issues...

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 15, 2020

Updated the PR so gl constants don't get replaced in non-minified builds.

All users who use a build system will now miss the constant replacement (since they use three.module.js).

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

Please consider #15176 (comment). I'm afraid the existence of three.module.min.js will cause a lot of issues...

Damn. I didn't think of that.
Yes, we can't do this until import maps are available, or we come up with an alternative.

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

All users who use a build system will now miss the constant replacement (since they use three.module.js).

I'll do some measurements to see what filesize savings these provide.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 15, 2020

I'll do some measurements to see what filesize savings these provide.

Question: Didn't you add the constant replacement primarily for performance reasons? I thought it would be faster to just embed the value instead of looking it up over the gl reference...

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

Question: Didn't you add the constant replacement primarily for performance reasons?

I think the original intent was filesize savings, but performance improvements was a side effect that I forgot. I guess I'll revert that too haha 😅

Although I know that sometimes when debugging your project you find yourself having to look up these constants. But I haven't heard anyone mentioning that recently.

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

Hmm, what a strange thing.

Screen Shot 2020-09-15 at 7 43 53 PM

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

Well, that fixed it. I don't even want to know why that happened.

@mrdoob
Copy link
Owner Author

mrdoob commented Sep 15, 2020

@Mugen87 looks good to you?

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 15, 2020

BTW: I've switched to Terser since a while now because of the easier rollup integration and made good experiences 👍 .

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.

3 participants