-
Notifications
You must be signed in to change notification settings - Fork 240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Notification system for cli updates #146
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# Notification system for cli updates | ||
|
||
## Summary | ||
|
||
A new notification system to warn users about new versions of the **npm cli**. | ||
|
||
## Motivation | ||
|
||
- Remove an extra http request from the cli | ||
- Remove dependency on `update-notifier` | ||
- Original discussion: https://github.com/npm/rfcs/discussions/118 | ||
|
||
## Rationale and Alternatives | ||
|
||
- Do nothing, keep the same system in place | ||
- More alternatives? | ||
|
||
## Implementation | ||
|
||
- Set a `user-agent` header in the requests from the **npm cli** | ||
- Add a check on the registry that notifies user if a greater version of the cli is available | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this needs to take into account the node version; there's very complex logic (https://github.com/nvm-sh/nvm/blob/master/nvm.sh#L150-L269) for figuring out which npm version works on which node version, and it would be pretty bad if npm suggesting updating on a node version where the act of updating completely hosed the entire node install by breaking npm. |
||
- Add a new header in the registry response that alerts for a new version of the cli | ||
- Also allow customization so that other registry clients can implement/benefit from the same system | ||
|
||
## Prior Art | ||
|
||
- https://www.npmjs.com/package/update-notifier | ||
|
||
## Unresolved Questions and Bikeshedding | ||
|
||
TBD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this makes sense. It is just one request per week. During an installation there are hundreds of requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my guess is that when this was brought up it was with regards to commands that should not necessarily hit the registry, such as
npm ls
but i do agree with your point, it might be silly and not worth having this listed in the RFC after allThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth it to keep it if reducing HTTP calls where they may not be necessary is a goal of the project (I think that's a fair and valid goal).