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

Create .npmignore #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

quinton-ashley
Copy link
Contributor

I think offering Box2D v3 at a lightweight size is one of the big advantages of this JS port! But the npm package is a hefty 12MB.

Therefore it might be best to use an .npmignore file so that only the dist folder and only other necessary files be included in the npm package, which is more typical. If users want to get everything else they can download the git repo.

@photonstorm
Copy link
Contributor

The npm size has zero relation to the bundle size. The tradeoff is purely convenience vs a relative small amount of disk space.

@quinton-ashley
Copy link
Contributor Author

quinton-ashley commented Jan 5, 2025

For web apps, yes. But it would have an impact on Electron app bundle size.

I suppose if people are concerned about making their apps smaller they can install phaser-box2d as a dev dependency and then copy the release build within their app folder as part of their build process.

Personally I wouldn't make users to do that. I'd only include the release build and minified release build in the package so users can simply use npm for package management. I don't think users need to build from source or read through the docs from within node_modules. Just a suggestion though.

@mreinstein
Copy link
Contributor

I don't see any upside to bloating the npm package. If it's not used at run time it probably doesn't belong in the npm module. Dev artifacts should stay in the git repo. Electron app bundle size is a valid example of this, and also CI/CD (not having to pull down 10s of extra megabytes adds up over thousands of runs. :) )

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