Skip to content
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

api.AbortSignal.timeout - node support in 17.3.0 #15884

Merged
merged 3 commits into from Apr 19, 2022

Conversation

hamishwillee
Copy link
Collaborator

From Node docs here https://nodejs.org/api/globals.html#static-method-abortsignaltimeoutdelay AbortSignal.timeout() is supported from 17.3.0.

This won't merge because node versions above 17.0.0 are not yet supported.

This follows on from the comment here: #15644 (comment)

@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Apr 19, 2022
@queengooborg
Copy link
Collaborator

Thanks @hamishwillee! Would you be up to add NodeJS v17.3.0 to the browser data in this PR (or another PR)?

@hamishwillee
Copy link
Collaborator Author

@queengooborg Well, its a little out of scope.

Happy to give it a go but where is the release information that is needed? I infer that we

@queengooborg
Copy link
Collaborator

These are great questions! I've been basing the engine version (and yes, the engine's always V8) and release date off of https://nodejs.org/en/download/releases/. The status should be set to "current" (whereas 17.0.0 should be set to "retired").

@hamishwillee
Copy link
Collaborator Author

@queengooborg So do you do it for all point releases, or just the ones you "need" - for example there is at least 17.1.0 and 17.9.0 - if there are ones in between do we do them too?

So the remaining question is - How do I work out engine version? That's not "V8" its a number like "engine_version": "9.4"

@queengooborg
Copy link
Collaborator

We only add the ones that are needed, typically on the following:

  • A new major release (v16, v17, etc.)
  • A release that changes the V8 engine version (v17.0.0-v17.1.0 uses 9.5, but v17.2.0 and on use 9.6)
  • A release that otherwise adds a feature, like in this case

So I would say, add both v17.2.0 and v17.3.0 since v17.2.0 changed V8 engine versions and v17.3.0 added this feature (confirmed by https://nodejs.org/api/globals.html#static-method-abortsignaltimeoutdelay).

As for the engine, I think there's a little bit of confusion -- the engine is actually called "V8" (like the V8 engine in a car, and definitely not the vegetable drink), I'm not saying "v8" like "version 8"! Based on the link above, it says that NodeJS v17.3.0 uses V8 v9.6.180.15 (which we shorten to "9.6"), so our statement should have the following:

"engine": "v8",
"engine_version": "9.6"

@hamishwillee
Copy link
Collaborator Author

Thanks for the detailed explanation @queengooborg - hopefully next time I will be able to work it out for myself!

@github-actions github-actions bot added the data:browsers 🌍 Data about browsers (versions, release dates, etc). This data is used for validation. label Apr 19, 2022
Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick changes! Just a small change: since v17.0.0 and v17.2.0 have been superseded by a new release, v17.3.0, we should mark them as retired status instead. Browsers (as well as NodeJS) should only have one "current" release.

browsers/nodejs.json Outdated Show resolved Hide resolved
browsers/nodejs.json Outdated Show resolved Hide resolved
Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <vinyldarkscratch@gmail.com>
@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Apr 19, 2022

@queengooborg Done. FYI only, note that I did have a reason for marking them as current - the blog marks them as such still : https://nodejs.org/en/blog/release/v17.1.0/

Note to self, ignore the blog :-)

Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that's kind of odd that the blog still says "current" for them... Ah well!

Thank you very much, this PR is LGTM! 👍

@queengooborg queengooborg merged commit 3db2dce into mdn:main Apr 19, 2022
@hamishwillee hamishwillee deleted the node_abortsignal_timeout branch April 19, 2022 08:01
@hamishwillee
Copy link
Collaborator Author

Awesome - and thanks for the education!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API data:browsers 🌍 Data about browsers (versions, release dates, etc). This data is used for validation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants