-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Adding a Version Checker #44942
Comments
Basic implementation: https://gist.github.com/flakey5/a719d0dc0ebdb08eaf479dc531ea13d8 Note this is just for experimenting and testing. Node coding conventions aren't followed. Errors don't fail silently for testing purposes. |
@nodejs/version-management Is this something any of the version managers ( |
Not that i know of - although npm validates engines.node. Recommending an update is a very dangerous thing to do, and i don’t think node should be doing it. |
How so? In regard to breaking changes, there would be an option for a user to limit update notifications to whatever major they're currently on with |
I might not mind it opt-in only, but also I'm not sure it couldn't then be an npm module. |
Just my $0.02: Recommendations are opinionated and should not be a part of core. This seems like something that a version manager could do, but who should be making recommendations? The "who" seems circumstantial... perhaps a team lead, a vendor, or a development standards body within an organization, etc. |
This might just be from my personal pov but I don't really consider this to be a recommendation in the first place. I would consider it more of just a convenient notification to inform the user that there's a new version available. If they want to update they can, otherwise they can just ignore it. |
This presumes every user is actually in control of their node version - if they work on a shared project, the chances they’re unilaterally responsible for updating node are slim - and there’s tons of convenient ways to get a notification when new node versions are released. In other words, I’m not sure why this is a problem worth solving. You can always run |
The check that @flakey5 is proposing would be opt in only and does nothing more than notify if a newer version is available.
The notification does not recommend an update, not does it perform the update. It allows a user to easily and optionally find out if there is an update. I'm not understanding how such an optional check is "very dangerous". Sure, the user might not be entirely in control of the version they use, but that's secondary.
Everything we add could be an npm module. The arg parser could have been (is) an npm module. The test runner could have been (is) an npm module. Deleting a directory recursively could have been (is) an npm module...etc. I'm finding myself less and less swayed by this line of reasoning. |
Fair point. it does, however, make a network call that’s presumably hardcoded to nodejs.org, in a way that the user may not be aware of due to env vars setting NODE_OPTIONS. I’m still very unconvinced that this is a problem worth solving. |
The proposal does use the URL that has been well known and used by many tools for many years and provides a simple way of overriding the URL. If setting the env vars is not ideal, initiating the check and overriding the URL can be limited to command line options. We can disagree on what problems are worth solving. I didn't really think we needed an arg parser, for instance. |
Sure, of course we can. However, args parsing constitutes extremely high usage on npm; does a node update notifier? |
We have no way of knowing really. How often do users check the website to see if there's a new version? How often do they use version managers to check? How often do they wait for notifications on Twitter? There's no way of speculating on it. In any case, I'm +1 on the change and hope it progresses. The concern over the env vars is valid and @flakey5 I recommend that this is limited to command line args to trigger the check and override the URL. |
Vendoring in |
If we could bring in semver and expose it that’d pay for itself a millionfold, so if this is the catalyst for that, go for It :-) (fwiw on Twitter you don’t have to wait for anything; you can get mobile notifications on a user that polls nodejs.org/dist) |
That is a good point. I'll update it to only work off of cli arguments to fix that.
The source files for semver are only ~57KB and the total size of the module is ~86KB |
We would need to make sure |
We should split those decisions re: semver. It's perfectly fine for us to vendor in the dependency for use here. Choosing whether to expose it as an API is a different question entirely that should be discussed separately. But yes, this check can be a path to getting there. |
This does sometimes get spammed while people are waiting for an update to become available. 😄 |
+1 if |
Out of topic but due to the example implementation: Is there an open discussion on adding |
That would be possible, yes, but it's a separate discussion that we should open a separate thread for. |
@anonrig there's a discussion here: nodejs/tooling#146 |
The OP says opt-in only. I see that was an edit, but IMO it's a good and non-controversial edit. I'm +1 this since it's explicitly opt-in.
FWIW semver is already included in Node.js via being included in npm, so if we added semver to Node.js core first, npm in Node.js could use Node.js's semver and we'd theoretically see zero bundle size change.
They are, they literally asked us to. It's the npm team. I don't want to speak for them, obviously, but my understanding is that they'd be fine. (I'll cease the semver talk here, just wanted to address these open questions)
unfortunately this user died when I quit Microsoft. Personal-time project that's not affiliated with Node.js that I'm slowly working rewriting on when I have energy :) |
One small note: I'd change |
Except you'd need to support people running newer versions of npm on older versions of node. That means having to keep two copies around for some time. |
The total of both copies would be under 200kb, so it wouldn't really be noticeable imo. Also, as of now the current edit represents the most up-to-date proposal. I'll start working on the pr today or within the next new days. With some thought on it I do think vendoring semver would help tremendously and, from what I can tell, it is wanted by others as well. I'll probably vendor it in a separate pr and when that lands I'll open the one for the version checker just for neatness. |
@flakey5 :
Since this will immediately exit after performing the check, we don't actually need
I'd suggest renaming this to |
I would recommend a bit more nuances on the rules above.
|
I'm unaware of consensus on that, and judging from the reaction of others here, I'm not alone. At a minimum, this would require some more conversation. ANYWAY, back to the original question or something close to it: Would it be good for Node.js to have an opt-in configuration of some kind that will let you know, maybe once a week or so, if you are running a version of Node.js that is either EOL or has a newer version available with the same major release number? My answer is: That would be a great experimental feature to add. Node.js doesn't need it, but I think it would help a lot of users, and it's kinda sorta expected for a lot of things these days. On the other hand, that tends to be something that shows up in tools more than runtimes. But still, as an opt-in experimental feature, I like that experiment. It could be an npm module we publish officially, and I wouldn't mind that either if that's the preferred route. |
When I tried to think of similar features in other tools, the only things I could come up with are auto-updating apps like Chrome; and What if instead of being a command, we followed the example of |
npm's update notifier has been the source of a very large number of complaints, and while I don't think there's a concerted effort yet to remove it, I'd be surprised if the npm team was particularly attached to keeping it. |
Agreed.
Please let's not, this will increase the cold boot time of node and be a huge source of complaints.
It would then be a very useless feature. |
It truly feels we all have very different opinions on how to do this. Yet it seems that other JavaScript runtimes do have built-in update commands. But no commands for checking if an update is available (besides of running the update command with This could indicate that adding such a command on Hence, adopting a solution like also telling the user when a new version of Otherwise, the website is a good source for notifying the user upfront with 0 interaction. Because either adding it on And what is the actual goal? How does adding this to Of course, that's not the case for minor and patch versions, as they include bug fixes, security patches and more. But if the reason for being "notified" is for such updates (because I believe that's the use case I'm most interested in), it should (definitely) be something in the line with "I want to be notified" as soon as it is out. And not something I need to remember to run every once in a while. Hence why I originally suggested But honestly, it feels like the best place for a "notify me for an update of node.js" on our website. |
@ovflowd :
hmm... I read this as: They don't have commands to check for an update except the commands they have that check for an update. The "dry run" options are specifically provided for exactly this kind of purpose. It's just implemented in a different way.
I don't understand how you can draw this conclusion. Other tools do in fact have mechanisms for checking to see if there is an updated version, the fact that those might be tied to additional functionality for actually performing the update is secondary in my opinion.
This just feels like quibbling over a definition. Very well, let's not call it a notification and let's not rabbit hole any further on whether this feature proposal is actually a "notification" or not. It's an explicit check to see if there is an updated version.
Speaking for myself, I regularly use node on multiple systems with potentially a broad range of Node.js versions deployed on them. In every case, I always check |
I believe you misunderstood what I meant. Either we add a command that allows
It is not. Either it is a "notification" or is the user manually checking for an update. That's not what a notification is IMHO 😅
I don't think the average user is aware of this. Which is something many pointed here out. The average user is not well-versed enough to understand these technicalities. Not saying they could, but then we're talking about user re-education.
You still need to go to a website to download a newer version of node or use Isn't then just easier to altogether just ask Again, I'm not opposed to this change; just really trying to ponder what are the actual benefits of doing just a version checker directly on node itself and for which audience we're making this change? |
I do think the idea is really interesting and can be useful to improve the DX. From what I see, the current way to "solve this issue" would be to
The proposed version would add a command to keep everything in the terminal
My takes on that would be that people not looking from time to time on the website to see new release won't check the version using the command line with another new command. But that could be interesting to still add the functionality. Maybe with some minor (?) changes
Also, I don't think this would be a great idea to have a 'notification' when you run node, or every week or something else. This could be a bit spammy or on the other hand just be completely hidden by systems (and so not being useful) |
"doing an update" is complex, given that not everyone has permissions to do so; not every environment will want to permit doing so even if the user has the permissions (think an infra team wanting to prevent devs from footguns); not everyone will have the disk space or the internet bandwith to do so successfully, so error recovery will be very critical; without certificate pinning, the nodejs.org connection could be intercepted, which could lead to security issues, and bundling certificates into node will mean that updating breaks in the future when those certs expire; etc. |
I think having a I can understand adding this now if we intend to add the ability for |
@GeoffreyBooth this is one of the points I was referring to. What problem are solving with this, and to who? |
Thanks for explaining! 🙏
As I mentioned in #44942 (comment) (second part) and #44942 (comment) (
How is this a common use case? Are there other issues asking for this feature? AFAICT, this feature can be implemented in userland but there is no such implementation maybe because this feature is not a common use case?
Are the other uses mentioned in this issue? Could you please share a link? If you're referring to there being a duplication of
Why would we expect that? I don't think any third part tool has implemented that. Sure, if it were to be implemented by a lot of third party tools, I can see your point but the fact here is that none of the third part tools have implemented this feature in spite of being capable of doing so. |
One use case might be to have people run |
Personally I feel like if we can’t find any examples out there of CLI tools with a similar feature, this is likely to be confusing to users and therefore something that we shouldn’t add. It makes a lot of sense to me as a component of an “autoupdate” feature, or even a user-triggered update feature, but if we’re not planning on adding such features then this “version check” on its own feels a bit like a loose end. Before going forward with “version check” on its own I would like to see examples of such a standalone feature elsewhere, so that I can see how the UX makes sense in similar tools; or a plan for what this feature would support as some kind of larger goal such as an “update” feature. |
Would it be possible for the existing Node.js version managers to use this feature? What would the interop story look like? Would they still need to implement a similar check themselves? I saw that there was a user survey in 2021, but couldn't find the data. Would be nice if we can check how people get their Node.js installation in the first place to decide which approach would benefit the majority of our users. |
A node version manager either has to bundle its own copy of node - which one or two do - or, more commonly, it wouldn’t ever be able to use a feature built into node, since it couldn’t rely on that feature, or node itself, existing. |
@flakey5 ... This was discussed on the TSC call and it's looking like there is no consensus currently on adding this feature. There was a good amount of discussion on the topic if you want to review the TSC call recording, but otherwise I think the current status here is that this is not likely to land at all. |
I personally see the points:
|
Alrighty, well, I think I'll close the issue now then. I'll keep the semver pr open since there are other use cases for it but I'll remove the reference to this issue. |
I just wanted to say (and I think everybody here agrees), @flakey5, is that we really appreciate your engagement and your contribution! Whilst this will not land for now, we do appreciate the effort you put into this. And I definitely welcome you to contribute with other Issues or things you believe will benefit the broader Node community. Ideas like these (spontaneous and cool) drive the project :) Stay safe! |
Thank you all as well for your time! |
What is the problem this feature will solve?
Node does not have a version checker that lets the user know if they're on an outdated version. In my opinion, having one would absolutely improve QOL and would help notifying users of new versions.
What is the feature you are proposing to solve the problem?
Adding a opt-in version checker. I propose it as opt-in because of potential privacy concerns and this could be considered a breaking change due to the additional overhead. Also, I believe it would be best to implement it on the Javascript side just for simplicity.
The Node.js website already has a file that is automatically updated with every release (https://nodejs.org/dist/index.json) so all changes that need to be made are in Node core itself.
--check-update
cli argument and you set it to what release branch you follow.--version
where it simply checks for an update and then exits.--check-update=current
- Follows the most current release, User will get notified for any new version.--check-update=lts
- Follows the most recent LTS release. It will skip any odd-numbered majors and only focuses on the latest release of a LTS version. Ex/ user runningv16.x.x
will get notified forv18.x.x
but not forv17.x.x
since it's not LTS--check-update=same
- Follows whatever major you are on and ignores any updates not pertaining to the major. Ex/ user runningv17.x.x
will get notified forv17.y.y
but not forv18.z.z
since it's not on the same major. The name for this isn't great, if someone has a better idea please say so. If the cli argument is passed with no value (--check-update
and nothing else), this is what it defaults to.--version-check-url
cli argument in case the user would like to override the url it requests for whatever reasonprocess.emitWarning
with a message such asNew version available! vX.X.X (current) -> vY.Y.Y (new)
. Could also add a note if it's a security release likeNew version available! vX.X.X (current) -> vY.Y.Y (new) (security release)
.I am currently working on implementing this and created this issue to open the discussion on it and get some feedback. I have a basic experimental implementation which is linked in the reply directly below this post.
What alternatives have you considered?
No response
The text was updated successfully, but these errors were encountered: