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

Update Node.js globalThis version #4043

Merged
merged 2 commits into from May 2, 2019
Merged

Conversation

andria-dev
Copy link
Contributor

@andria-dev andria-dev commented May 1, 2019

Fixes #4041

Changes

  • Updated version when globalThis was added in Node.js
  • Added v12.0.0 to Node.js versions file

Verification

I checked on v11.15.0 (latest on v11) where globalThis doesn't exist. I also checked on v12.0.0 (next after v11.15.0) where globalThis does exist. I couldn't find information in the changelogs or where the feature was actually introduced in the code but here are some screenshots:

v11.15.0
Screen Shot 2019-05-01 at 3 22 29 PM

v12.0.0
Screen Shot 2019-05-01 at 3 27 17 PM

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

LGTM, r+

P.S.: The failing build is unrelated to this PR and is reproducible on master.

@queengooborg
Copy link
Collaborator

Sorry about the failing test! As @ExE-Boss mentioned, it's nothing to do with this PR -- we had a troublesome PR merged. I just commited a fix for this, if you merge the master branch to this one, the test should now pass. 😉

@queengooborg queengooborg added data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API rebase-needed 🚧 labels May 2, 2019
@andria-dev
Copy link
Contributor Author

Just rebased 👍

@queengooborg queengooborg added data:browsers 🌍 Data about browsers (versions, release dates, etc). This data is used for validation. and removed rebase-needed 🚧 labels May 2, 2019
@queengooborg
Copy link
Collaborator

Thank you! This PR's looking great to me -- it's weird that it's not included in the release notes for v12.0.0, but the screenshots provide enough evidence, I'd say.

👍 from me!

@queengooborg queengooborg merged commit 6f84c37 into mdn:master May 2, 2019
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.

globalThis is now supported in Node.js
3 participants