-
-
Notifications
You must be signed in to change notification settings - Fork 307
Description
First off, I love this package. Being able to fix a bug in a dependency, while waiting for the PR to be merged, is awesome for productivity. Great work and thank you!
Anyway, I was wondering if it's possible to set update-notifier
as an optional dependency? To be honest, I'm wondering why it's included.
It's not very useful when running via npx
, since that gets you the latest version of patch-package anyway. It's also not that useful when installing patch-package locally in a project, since there usually is some sort of dependency update convention (ie: not willy-nilly); and a quick yarn upgrade-interactive --latest
will pull in the latest patch-package anyway. Also, the update-notifier dependency isn't without its own dependency tree (http://npm.broofa.com/[email protected]), so this would also save install time, disk space and startup cost.
I can imagine it to be useful if patch-package were installed globally, like npm
. But, with the advent of nvm
and other node version managers (where globally installed packages are lost upon a version switch), and the ease of npx
, it's increasingly less useful to install a package globally. Also, I can imagine this could create conflicts if a patchfile was made with an earlier version of patch-package.
The proposed minimal changes would be to move update-notifier
to optionalDependencies
in the package.json
file, and to put the actual require in index.js
in a try-catch. Since it checks for isCI
anyway, this would be a small addition.
Would you be open to this change? If so, I would be willing to make a PR.