Suggestion for module check: "npm install --omit=dev" #53
Description
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.
Activity
sdetweil commented on Apr 17, 2025
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 commented on Apr 17, 2025
any user that want to do dev can rerun npm install
99.999% dont need dev
dathbe commented on Apr 17, 2025
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.
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 commented on Apr 17, 2025
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 commented on Apr 17, 2025
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 commented on Apr 17, 2025
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 vulnerabilitiessdetweil commented on Apr 17, 2025
didn't say vulnerability said dependency
KristjanESPERANTO commented on Apr 21, 2025
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).
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 commented on Apr 22, 2025
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 commented on Apr 22, 2025
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 commented on Apr 22, 2025
We also need to check that there actually ARE devDependencies in the package.json before throwing this warning.