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

Lighter pakage.json dependencies (remove puppeteer by default) #19332

Closed
munrocket opened this issue May 10, 2020 · 5 comments · Fixed by #19367
Closed

Lighter pakage.json dependencies (remove puppeteer by default) #19332

munrocket opened this issue May 10, 2020 · 5 comments · Fixed by #19367

Comments

@munrocket
Copy link
Contributor

Description of the problem

Some of the developers cloning three.js repo without the intention to contribute. This is happening because they want to make tree-shake library or modify core for your own purposes in projects.

The idea is to introduce to two additional commands and suggest in console to install puppeteer only those developer who want to take screenshots in PRs.

"make-install": "npm i -D puppeteer@2.1.1 pngjs pixelmatch image-output serve-handler failonlyreporter",
"make-clean": "npm un puppeteer pngjs pixelmatch image-output serve-handler failonlyreporter",

But by doing so we modifying package.json after npm run make-install and facing another problem: we need to say to contributors not to add package.json or package-lock.json, which is relatively easy but little bit uncomfortable. I wonder if exist any safe way to keep package.json the same after install.

This idea is consistent with #19321 #19326

@donmccurdy
Copy link
Collaborator

An option would be to put a package.json for the screenshot tool into a subdirectory. Users who need to run it can run npm install in that directory; others would not. Dependencies from the parent folder are automatically available in the subdirectory.

@munrocket
Copy link
Contributor Author

The solution was simple. Need to rethink this, +150mb is annoying.

@munrocket
Copy link
Contributor Author

Ty.

@munrocket
Copy link
Contributor Author

Solved, sanks.

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 a pull request may close this issue.

3 participants
@donmccurdy @munrocket and others