[cli] Replace update-notifier dependency with built in
#8090
Conversation
styfle
changed the title
update-notifier dependency with build inupdate-notifier dependency with built in
| 'application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*', | ||
| }; | ||
|
|
||
| const url = `https://registry.npmjs.org/${pkg.name}`; |
There was a problem hiding this comment.
Perhaps make url configurable so we could test against a mock server?
| `Changelog: https://github.com/vercel/vercel/releases/tag/vercel@${latest}` | ||
| ) | ||
| ); | ||
| console.log( |
There was a problem hiding this comment.
We should be using output.log() here instead of console.log() otherwise we'll break the pipe-ability of commands (i.e. deployment URL goes to stdout for vc deploy).
I realize that this was the previous behavior as well, but since we're here we might as well fix it.
There was a problem hiding this comment.
Because of the isTTY check, this won't break piped output, but by convention and for future testing, we should still probably change these to use output in a follow-up PR.
There was a problem hiding this comment.
Can we update this in a follow-up PR?
This was about using output instead of console.log.
| 'update-notifier', | ||
| 'src/index.ts', | ||
| ]; | ||
| console.log('Dependencies:', dependencies); |
There was a problem hiding this comment.
Can we update this in a follow-up PR?
We probably don't want to log if dependencies is empty. Or, at least we'd want to log a clearer message saying so.
| `Changelog: https://github.com/vercel/vercel/releases/tag/vercel@${latest}` | ||
| ) | ||
| ); | ||
| console.log( |
There was a problem hiding this comment.
Can we update this in a follow-up PR?
This was about using output instead of console.log.
This PR replaces the `update-notifier` dependency with a custom implementation. There are a few reasons: the dependency is quite large, it requires ESM in order to update, can sometimes suggest an update to an older version, and used dependencies with known security issues. The result looks like: <img width="768" alt="image" src="https://user-images.githubusercontent.com/97262/208452226-b7508299-f830-4d42-a96a-7646ec8227aa.png"> Note: This PR is the successor to #8090.
This PR replaces the
update-notifierdependency with a custom implementation.There are a few reasons: the dependency is quite large, it requires ESM in order to update, and can sometimes suggest an update to an older version. For example:
update-notifierto fix vulnerability #8038