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

Drop support of Node v10 #4633

Merged
merged 1 commit into from
May 21, 2021
Merged

Drop support of Node v10 #4633

merged 1 commit into from
May 21, 2021

Conversation

juergba
Copy link
Contributor

@juergba juergba commented May 7, 2021

Description of the Change

We drop support of Node v10 with end-of-life: 2021-04-30

@juergba juergba added area: node.js command-line-or-Node.js-specific qa semver-major implementation requires increase of "major" version number; "breaking changes" labels May 7, 2021
@juergba juergba requested a review from a team May 7, 2021 14:13
@giltayar
Copy link
Contributor

giltayar commented May 7, 2021

Can we make the engine minimum be 12.7.0? That version is the minimum for esm compatibility, which means that we can drop all kinds of ifs in the tests and the code around that.

I'd love to submit a pr around that, which should be part of this semver major release.

Copy link
Contributor

@giltayar giltayar left a comment

Choose a reason for hiding this comment

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

I'd love to change the way we require/import tests files to always import, now that all the node versions support esm.

I can create it as a separate pr, but it should be part of the v9.0.0 version, as it could (very theoretically) break things.

@juergba juergba added this to the v9.0.0 milestone May 7, 2021
@juergba
Copy link
Contributor Author

juergba commented May 7, 2021

Can we make the engine minimum be 12.7.0?

Yes, but in utils.supportsEsModules you are checking for >=v12.11. Now we take 12.7.0, sure?

I can create it as a separate pr [...]

Please, go ahead then. We don't have enough content for a serious semver major, yet.

@giltayar
Copy link
Contributor

giltayar commented May 8, 2021

@juergba I'll get back to you on the minimum number. I'll check it while building the PR.

@giltayar
Copy link
Contributor

giltayar commented May 8, 2021

I checked, and the version that finalized support for ESM was 12.17.0 (I confused 7 and 17). Note that the version that removed the "experimental ESM" warning was 12.20.0, which is a pretty late v12 version (the latest v12 version is 12.22.1).

This means that if I implement the PR that removes the "require and if fails then import", anybody using v12 that is earlier than v12.20.0 will get a warning from ESM. I'll go ahead and implement it anyway, and we can discuss this on that PR.

@juergba
Copy link
Contributor Author

juergba commented May 21, 2021

@giltayar I'm going to merge this PR, you can adjust the minimum version later on.
I hope you are alright in your country in these difficult times.

@juergba juergba merged commit 641970d into master May 21, 2021
@juergba juergba deleted the juergba/drop10 branch May 21, 2021 09:25
@giltayar
Copy link
Contributor

I would love it if we could add this new PR to the next semver-major version of Node.js, and before the semver-major version is launched: #4635. This PR could only have been done when we dropped Node v10. The reasons are outlined in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: node.js command-line-or-Node.js-specific semver-major implementation requires increase of "major" version number; "breaking changes"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants