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.

Sign up for GitHub

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

Jump to bottom

Add three/addons target for NPM. #26910

Merged
merged 9 commits into from
Oct 17, 2023
Merged

Add three/addons target for NPM. #26910

merged 9 commits into from
Oct 17, 2023

Conversation

CodyJasonBennett
Copy link
Contributor

@CodyJasonBennett CodyJasonBennett commented Oct 7, 2023 •
edited
Loading

Related issue: #24595

Description

Tries to pick up after #23368 which was closed due to lack of tree-shaking which we've found unnecessary with #26912.

@CodyJasonBennett CodyJasonBennett marked this pull request as draft October 7, 2023 01:56
@github-actions
Copy link

github-actions bot commented Oct 7, 2023 •
edited
Loading

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
652.2 kB (161.6 kB) 652.2 kB (161.6 kB) +0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
444.9 kB (107.6 kB) 444.9 kB (107.6 kB) +0 B

@CodyJasonBennett CodyJasonBennett mentioned this pull request Oct 7, 2023
Closed
LeviPesin
LeviPesin reviewed Oct 7, 2023
View reviewed changes
examples/jsm/Addons.js Outdated Show resolved Hide resolved
CodyJasonBennett added a commit to CodyJasonBennett/three.js that referenced this pull request Oct 7, 2023
@CodyJasonBennett
offscreen: revert changes
4654d8e 
See mrdoob#26910 (comment)
@CodyJasonBennett CodyJasonBennett mentioned this pull request Oct 7, 2023
Merged
@CodyJasonBennett CodyJasonBennett changed the title Add three/addons target Add three/addons target for NPM Oct 8, 2023
@CodyJasonBennett CodyJasonBennett changed the title Add three/addons target for NPM Add three/addons target for NPM. Oct 8, 2023
@CodyJasonBennett CodyJasonBennett marked this pull request as ready for review October 8, 2023 13:13
This was referenced Oct 9, 2023
Closed
Closed
@bhouston
Copy link
Contributor

Amazing work! @CodyJasonBennett !

marcofugaro
marcofugaro reviewed Oct 16, 2023
View reviewed changes
examples/jsm/Addons.js Outdated Show resolved Hide resolved
examples/jsm/Addons.js Show resolved Hide resolved
examples/jsm/utils/BufferGeometryUtils.js Outdated Show resolved Hide resolved
@marcofugaro
Copy link
Contributor

Alright, this PR looks good to me 👍 ready to be merged @mrdoob @Mugen87

After further testing, we found out that this PR is already tree-shakeable by iteself and doesn't need the changes from #26912.

@Mugen87 Mugen87 added this to the r158 milestone Oct 16, 2023
@CodyJasonBennett CodyJasonBennett mentioned this pull request Oct 16, 2023
Merged
@Mugen87 Mugen87 merged commit de4d261 into mrdoob:dev Oct 17, 2023
19 checks passed
@CodyJasonBennett CodyJasonBennett deleted the npm/addons-target branch October 17, 2023 18:21
@CodyJasonBennett
Copy link
Contributor Author

Thanks to @marcofugaro for the sanity check and for championing this as much as me.

This was referenced Oct 18, 2023
Merged
Merged
@Methuselah96 Methuselah96 mentioned this pull request Oct 22, 2023
Closed
16 tasks
@CodyJasonBennett CodyJasonBennett mentioned this pull request Oct 23, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeviPesin LeviPesin LeviPesin left review comments

@marcofugaro marcofugaro marcofugaro left review comments

@Mugen87 Mugen87 Mugen87 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r158
Development

Successfully merging this pull request may close these issues.
5 participants
@CodyJasonBennett @bhouston @marcofugaro @Mugen87 @LeviPesin