Run npm install as part of install.py

[sbc100]

No description provided.

[dschuff]

Does this mean we can omit from build.py? it looks like that one is using 'npm ci'... so what is this part for?

[sbc100]

Does this mean we can omit from build.py? it looks like that one is using 'npm ci'... so what is this part for?

Exactly yes.

[dschuff]

What's the difference between 'npm install' and 'npm ci' then?

[sbc100]

What's the difference between 'npm install' and 'npm ci' then?

npm ci always uses the exact pinned versions I believe... its designed to be reproducible.

[dschuff]

So I guess switching from that to this would be a functional change then. If that's fine, then this PR LGTM. I guess it should be fine to just let it run twice for the few versions after this rolls into emscripten-releases and before we remove the one there?

[sbc100]

The emscripten-releases build script is already using npm ci so it would not be a functional change. --omit=dev is the another (more modern) way to spell '--production'`

[sbc100]

ping..

[dschuff]

The emscripten-releases build script is already using npm ci so it would not be a functional change. --omit=dev is the another (more modern) way to spell '--production'`

Maybe I'm misunderstanding what the intention is then. If we put this into install.py and then remove npm ci from build.py it would be a functional change, right? Or is the intention to keep using the one in build.py, and running both?

[sbc100]

The emscripten-releases build script is already using npm ci so it would not be a functional change. --omit=dev is the another (more modern) way to spell '--production'`

Maybe I'm misunderstanding what the intention is then. If we put this into install.py and then remove npm ci from build.py it would be a functional change, right?

Can you explain why you think its a functional change? We only need to run npm ci once, and running it twice should be a no-op.

Or is the intention to keep using the one in build.py, and running both?

My intention is to remove the one from build.py yes.

[dschuff]

oh... uh, nevermind I misread the code, I was thinking we ran literally 'npm install' but it says 'npm ci' ... oops :)
Maybe we could update the PR description ?

[sbc100]

Ooops, sorry I had auto-commit on.