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

avoid checking in node_modules directories. #7538

Closed
sbc100 opened this issue Nov 20, 2018 · 3 comments
Closed

avoid checking in node_modules directories. #7538

sbc100 opened this issue Nov 20, 2018 · 3 comments
Labels

Comments

@sbc100
Copy link
Collaborator

sbc100 commented Nov 20, 2018

This is somewhat related to #5774, but can be done as an intermediate cleanup step.

We currently have two separate node_modules directories checked-in under emscripten:

./tools/node_modules
./tools/eliminator/node_modules

We also use the top level node_modules directory for installing ws which is only used to the socket tests.

After discussing this with @jgravelle-google and @mathiasbynens I'd like to propose that we switch to using npm to install all this stuff at the top level and remove the checked in versions.

As an initial cleanup I already uploaded: #7536

In order to avoid requiring developers to manually run npm install after cloning (at least initially) I'm proposing to run this transparently from emcc's check_sanity if needs be. A bit like how we build libc on demand behind the scenes.

emsdk users should never have to run npm install since the SDK should alwasy include the correct versions.

@kripken
Copy link
Member

kripken commented Nov 20, 2018

Sounds like a noticeable change could be that non-emsdk users would need to install npm (and not just node, although the official downloads contain both)?

@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 20, 2018

I'm pretty you node always comes with npm... but that it something I will verify and mention on them mailing list.

sbc100 added a commit to sbc100/emslave that referenced this issue Nov 30, 2018
That plan is to start using this top level node_modules
directory for any/all node module dependencies.

Ideally the SDK version would with pre-downloaded node_modules
so that users don't need to download stuff on first use.

See emscripten-core/emscripten#5774
See emscripten-core/emscripten#7538
@stale
Copy link

stale bot commented Nov 20, 2019

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Nov 20, 2019
@stale stale bot closed this as completed Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants