Switching from yarn to npm

[julienben]

Following our conversation in #2327, I did some quick benchmarking on yarn vs npm (3 runs each, clean node_modules, with both lock files present). Yarn was faster but only by 10-12 seconds.

IMO, this switch still makes sense. First, for the reasons discussed earlier (audit and ci) but also because it just makes our code and docs more consistent. Some of our commands require yarn (mostly installing dependencies) and while everything can be done with yarn, our docs go back and forth and are overwhelmingly npm-oriented.

I took advantage of this to refactor setup.js. We were using parseFloat to check node and npm versions. This was inaccurate since our requirement is 8.10 and parseFloat will return 8.1. We're now using a semVer comparison library.

[coveralls]

Coverage remained the same at 100.0% when pulling d4b07d3 on switch-to-npm into fc4dc29 on dev.

[jwinn]

@julienben Thank you for taking the time to do this. Great work! 👍

A few thoughts/questions:

Should appveyor.yml use npm ci instead of npm install?

Regarding npm audit:

• While out of this project's direct control, I get https://nodesecurity.io/advisories/598 referenced 9 times, related to tunnel-agent used by image-webpack-loader--necessitating image-webpack-loader to be updated with >=tunnel-agent@0.6.0.
• Should we think about having npm audit as part of the commit pipeline?

Should docs/general/faq.md be updated?

• I'm using Node v0.12 and the server doesn't work? -- Is this really relevant anymore? If it is, should we change the language to be more along the lines of "We settled on supporting the LTS and Current versions of Node.js for the boilerplate..."
• Use CI with bitbucket pipelines -- Should npm install be changed to npm ci?

[julienben]

I'm using Node v0.12 and the server doesn't work? -- Is this really relevant anymore? If it is, should we change the language to be more along the lines of "We settled on supporting the LTS and Current versions of Node.js for the boilerplate..."

Good catch. The "engines" entry in package.json will take care of warning users on old Node.js versions and we also have version checks before install and before the setup script. IMO, this is more than enough and makes this question irrelevant.

Use CI with bitbucket pipelines -- Should npm install be changed to npm ci?

Yup. Changing that.

Should appveyor.yml use npm ci instead of npm install?

That too.

Regarding npm audit

Can you expand on what you mean by having audit as part of the commit pipeline? Are there other projects that do this? How does it work?

[julienben]

(Note that Travis already uses npm ci by default.)

[jwinn]

Travis handles it nicely, let's keep AppVeyor using npm install until the LTS npm version changes to >= 5.7.

Can you expand on what you mean by having audit as part of the commit pipeline? Are there other projects that do this? How does it work?

Was just a thinking out loud; there aren't a lot of other projects, I've seen/know about, leveraging npm audit in any automated manner.

LGTM 👍 Great work on this.

[justingreenberg]

with respect to npm audit, as of v6.4 audit can be configured to fail based on vuln score, eg. npm audit --audit-level=high will exit 0 if only low or moderate level vulnerability is detected. this should resolve the issue with current deps, however this would also mean dropping support for npm v5.x so it may not make sense at this time

we can add audit in future release @julienben awesome work

[julienben]

Good to know about npm audit! Let's remember for whenever npm v6.4 ships with node LTS.

@jwinn I'll restore appveyor to using npm install in the babel fixes branch.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.