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

chore: drop support for Node.js v10 #252

Merged
merged 3 commits into from
Sep 14, 2021
Merged

Conversation

theoludwig
Copy link
Member

What changes did you make? (Give an overview)

  • Drop support for Node.js v10
  • Add Node.js v16 to the node-version matrix in CI with GitHub Actions

Is there anything you'd like reviewers to focus on?

BREAKING CHANGE: Minimum supported Node.js version >= 12.0.0

@voxpelli
Copy link
Member

We may want to require one of the later Node 12 releases, to be able to use all the newest features in it, just like we did with Node 10

@theoludwig
Copy link
Member Author

We may want to require one of the later Node 12 releases, to be able to use all the newest features in it, just like we did with Node 10

Yes, do you have a specific Node.js v12 version in particular we could require ?

@voxpelli
Copy link
Member

I tried finding a good comparison, but eg https://node.green/ has no easy way to just compare two versions, so right now I don't know if there's any substantial new addition language wise.

And haven't checked the Node.js API additions at all.

@theoludwig
Copy link
Member Author

theoludwig commented Aug 18, 2021

I tried finding a good comparison, but eg node.green has no easy way to just compare two versions, so right now I don't know if there's any substantial new addition language wise.

And haven't checked the Node.js API additions at all.

Right, I think it is not necessary to require a newer Node.js v12 version if we don't need it.
I think it is fine to require Node.js >=12.0.0.

EDIT: To ease the migration from CommonJS to ESM, I suggest to support these Node.js versions: "^12.20.0 || ^14.13.1 || >=16.0.0", so we will be able to import builtin modules like fs etc. like this : import fs from 'node:fs'.
Ref: sindresorhus/meta#15 (comment)

@theoludwig theoludwig force-pushed the chore/drop-nodejs-v10-support branch from 629c023 to 38806db Compare August 18, 2021 10:36
@theoludwig theoludwig mentioned this pull request Sep 5, 2021
4 tasks
Copy link
Member

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

Some thoughts, sorry for the slow review, I will try to be quicker 🙈

.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

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

Great! 👍

Since this is a breaking change I think that we should also convert to ESM before cutting the next release. I can submit a PR unless someone else is working on it ☺️

Co-authored-by: Linus Unnebäck <linus@folkdatorn.se>
@voxpelli
Copy link
Member

@LinusU We had planned doing that for 16.0.0: https://github.com/standard/standard-engine/milestone/2

@voxpelli
Copy link
Member

Focusing on this for 15.0.0: https://github.com/standard/standard-engine/milestone/1

@theoludwig
Copy link
Member Author

Great!

Since this is a breaking change I think that we should also convert to ESM before cutting the next release. I can submit a PR unless someone else is working on it

Yes, you can open a PR. 😄
But as @voxpelli said, currently it is planned for v16 to avoid too much BREAKING CHANGE in one single release (v15), but we can discuss that again in #251.

@theoludwig theoludwig merged commit 581a9d7 into master Sep 14, 2021
@theoludwig theoludwig deleted the chore/drop-nodejs-v10-support branch September 14, 2021 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants