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

Remove non ES6 builds. #25341

Closed
wants to merge 1 commit into from
Closed

Remove non ES6 builds. #25341

wants to merge 1 commit into from

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jan 27, 2023

Related issue: -

Description

This PR removes the non ES6 builds three.js, three.min.js and three.cjs.

I have originally suggested to keep the builds a bit longer, but with examples/js gone, these builds admittedly have lost their significance. Furthermore, I frequently see users at the forum or stackoverflow mixing UMD builds with modules from examples/jsm. This could easily be avoided by not having the above builds in the first place.

The PR also cleans up the remaining references to the UMD builds. It also removes the benchmark tests like suggested in #25124 (comment) which can't be properly converted to ES6 modules anyway.

@drcmda
Copy link
Contributor

drcmda commented Jan 27, 2023

this pr would make the library unusable in most environments it's being used in, there's no migration path and this is immediately breaking.

if you force this to be esm, it means you can't use three in anything that touches cjs, which is practically everything. this would require any other package in the end users project to be esm, which is impossible as cjs won't go away until browser esm is feature compatible, which it is not.

the simplest use case is react. it can't go esm because esm can't dynamically load imports in sync (for instance dev and prod builds), that's ~20% of three npm userbase. or @brunosimon threejs journey going down.

i kindly ask you to not do this. @mrdoob this is we talked about a while ago. if your wish for threejs to reach as many people as possible then this will throw most of them under the bus for no warranted reason at all.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 27, 2023

if you force this to be esm, it means you can't use three in anything that touches cjs,

I guess this needs more background information since I'm not familiar with the non-esm workflows.

How is it possible to use three.cjs and the ES6 modules from examples/jsm in combination? Isn't that a mismatch in the first place?

Besides, would it be sufficient to keep just three.cjs and not the two other ones?

@Mugen87 Mugen87 marked this pull request as draft January 27, 2023 11:18
@CodyJasonBennett
Copy link
Contributor

I guess this needs more background information since I'm not familiar with the non-esm workflows.

TLDR; this breaks dual-packages and large swaths of the ecosystem which cannot go ESM-only as a response to this PR, and libraries with a peer-dependence on three.js have no option to bundle or modify three.js in any way.

How is it possible to use three.cjs and the ES6 modules from examples/jsm in combination? Isn't that a mismatch in the first place?

It's not, people have to use pmndrs/three-stdlib over examples/jsm which does a lot more like transpilation, tree-shaking/dependency fixes (except #23372), and flatbundles. I've been trying to backport things here since the move to "addons".

Besides, would it be sufficient to keep just three.cjs and not the two other ones?

three.cjs is the only concern AFAIK for Node/NPM. Configuration-wise, this needs integration testing of course, but I don't see any change there if package.exports is configured as before; that takes precedence in modern Node versions.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 27, 2023

If three.cjs is the main issue here then I'll file a different, smaller PR that removes only build/three.js and build/three.min.js.

My main goal is to avoid confusion between UMD and ES6 for devs who are not using a build tool.

@HarbAlarm
Copy link

Sorry for bumping this, but if that's right we will in future only have the options of importing via ES6, using build tools or going with release r156 and older?
ES6 and npm have only brought problems to anything I set up, so I'd be thankful to keep normal builds around.

@marcofugaro
Copy link
Contributor

marcofugaro commented Sep 12, 2023

@HarbAlarm the available builds will be ES Module and CommonJS, that's what the whole ecosystem is moving forward to.

@HarbAlarm
Copy link

@marcofugaro welp, guess i'll have to stick with r156, thanks for the reply o/

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.

None yet

5 participants