Suggestion for module check: "npm install --omit=dev"

[dathbe]

I was just recently setting up a new mirror and had to reinstall all modules. I noticed that many had security warnings on npm install, and when I looked deeper, some could be avoided by not installing the dev dependencies. Given that most users don't dev, should we include a check that suggests to maintainers that they include npm install --omit=dev in their install and update instructions to avoid unnecessary dependency installs?

We could even go so far as to only offer this suggestion when there is a devDependencies section in the package.json

Maybe that gets too complicated given that we then need to include a dev section at the end that explains to install the dev dependencies.

[sdetweil]

did you use my backup/restore scripts?

in restore i check to see if there is a dev section, and comment it out. as --omit=dev STILL CHECKS their dependency tree AND puts out ALL the warning, but installs no files. stupid
also same in upgrade script

[sdetweil]

any user that want to do dev can rerun npm install
99.999% dont need dev

[dathbe]

did you use my backup/restore scripts?

in restore i check to see if there is a dev section, and comment it out. as --omit=dev STILL CHECKS their dependency tree AND puts out ALL the warning, but installs no files. stupid also same in upgrade script

I did not. I like doing it manually so I can do things like check for vulnerabilities and post PRs to fix. I can also clean up my own configs.

any user that want to do dev can rerun npm install
99.999% dont need dev

I agree with this. And, frankly, even if I'm dev'ing someone else's module, I'm probably not doing the linting. So even when I'm dev'ing, I probably don't need the dev dependencies. So it may be more like 99.9999%

[sdetweil]

post vulnerability, just be careful, no guarantees the FIX is an improvement . if it didnt get dragged in w the tree I'm doing there is some issue remaining. best to leave it alone

MagicMirror is not a public website

[dathbe]

Some issues are easy by bumping dependencies by a point release. But I ran into one yesterday where a module was dependent on an OLD version (multiple major versions behind) of cron. The new version did require some tweaking of the module because there were breaking changes, but it was easy enough to propose an update, which now works fine.

And you're right that it's not entirely necessary to by hyper-vigilant about security warnings on an intranet resource like MM. But it teaches me about code to squash these things.

Back a couple comments, are you sure that --omit=dev still checks vulnerabilities in uninstalled dependencies? I hadn't seen that. I'm going to check now.

[dathbe]

I find that --omit=dev does NOT check vulnerabilities of dev dependencies.

From the same package.json file:

$ npm install --omit=dev

added 2 packages, and audited 3 packages in 8s

found 0 vulnerabilities

$ npm install 

added 123 packages, and audited 124 packages in 15s

34 packages are looking for funding
  run `npm fund` for details

1 high severity vulnerability

To address all issues, run:
  npm audit fix --force

Run `npm audit` for details.

[sdetweil]

didn't say vulnerability said dependency

[KristjanESPERANTO]

If you want to suppress all vulnerability messages, you can do this by switching off the audit: --audit=false. But I'm not sure we should recommend it, even MM is not a public website.

---

Instead of npm install, npm ci is used in some modules. This has at least the advantage that the package-lock.json is not changed during the installation and therefore avoids problems with updating the module via git pull later on.

--audit=false and --omit=dev are also working with npm ci.

If we add a check, it should allow both (npm install and npm ci).

---

Maybe that gets too complicated given that we then need to include a dev section at the end that explains to install the dev dependencies.

There are already modules that have a dev section that also lists how to install the devDependencies. Here is an example.

---

Would you like to try writing this check? 🙂

[dathbe]

I definitely don't want to suppress vulnerability messages. I'd rather try to fix them. But I would like to avoid both unnecessary installs and unnecessary vulnerabilities if I can.

I agree with allowing for npm install --omit=dev and npm ci --omit=dev

I was trying to read up on install vs. ci yesterday and it didn't make a whole lot of sense. I gather that ci just looks at package-lock, but does that make package.json entirely unnecessary?

[KristjanESPERANTO]

I would categorize it like this: npm install is for developers modifying a module. npm ci ensures users get exacly the dependency versions which the developer tested. This ensures avoiding unexpected issues from dependency changes.

The package.json is relevant for npm install and does not only contain the dependencies, there are also other things like scripts, keywords and other metadata.

[dathbe]

We also need to check that there actually ARE devDependencies in the package.json before throwing this warning.