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

Fix tree shaking being too aggressive #289

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

Vrixyz
Copy link
Contributor

@Vrixyz Vrixyz commented Jul 22, 2024

Fix #278

@Vrixyz Vrixyz requested a review from sebcrozet July 22, 2024 15:36
@Vrixyz Vrixyz changed the title add more files to side effects Fix #278 Jul 22, 2024
@Vrixyz Vrixyz changed the title Fix #278 Fix tree shaking being too aggressive Jul 22, 2024
Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Can you please check if there is any package difference (with our compat and non-compat builds) with and without that change?

Rapier’s packages is already know for being quite big so I wouldn’t want to make it even bigger just to fit one bundler (in which case we would have to figure out a way to make this a special-case rather than apply to all the bundlers).

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Jul 26, 2024

Here is the diff while running testbed3:

image image

It might be interesting to consider running on a smaller project, from my understanding that's where threeshaking could be useful: removing functions detected as not used.

Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks. Ready to merge after the changelog is updated.

@Vrixyz Vrixyz merged commit 50223ff into dimforge:master Jul 29, 2024
4 checks passed
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.

Built app WASM issue after upgrading to 0.13.1
2 participants