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: add engines.node to package.json #227

Merged
merged 2 commits into from
Jan 7, 2023

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jan 4, 2023

What's the problem this PR addresses?

v0.11.0 (specifically #100) dropped support for Node.js v12 and raised the bar to Node.js v14.14.0 due to

return rm(path, {recursive: true, force: true});
(https://nodejs.org/dist/latest-v18.x/docs/api/fs.html#fspromisesrmpath-options) but this wasn't written down as far as I can tell.

Addresses #100 (review)

How did you fix it?

Added engines.node to package.json to document the minimum version.

"module": "commonjs",
"resolveJsonModule": true,
"skipLibCheck": true,
"strict": true,
"target": "es2017"
"target": "ES2020"
Copy link
Member Author

@merceyz merceyz Jan 4, 2023

Choose a reason for hiding this comment

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

According to https://node.green/#ES2020 Node.js v14.14.0 doesn't fully support ES2020 but we're not using spread parameters after optional chaining so that's fine, if we ever do use it the CI will catch it assuming v14 doesn't update to a version of v8 that supports it.
In a future PR I'll ensure spread parameters after optional chaining is transpiled.

@@ -22,7 +22,7 @@ module.exports = {
loader: `ts-loader`,
options: {
compilerOptions: {
module: `es2020`,
module: `ES2020`,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should read from tsconfig.json here to keep them in sync?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea however my next PR will replace webpack with esbuild so they shouldn't have time to go out of sync.

@aduh95 aduh95 merged commit 84e6e1d into nodejs:main Jan 7, 2023
@merceyz merceyz deleted the merceyz/chore/engines branch January 7, 2023 11:14
@merceyz merceyz mentioned this pull request Jan 7, 2023
sounisi5011 added a commit to sounisi5011/package-version-git-tag that referenced this pull request Mar 24, 2023
Corepack is available for all currently supported Node.js.
see nodejs/corepack#227

Therefore, we stop installing each package manager manually and enable Corepack in all Node.js environments.
sounisi5011 added a commit to sounisi5011/package-version-git-tag that referenced this pull request Mar 24, 2023
* 💚 Using Corepack with all Node.js

Corepack is available for all currently supported Node.js.
see nodejs/corepack#227

Therefore, we stop installing each package manager manually and enable Corepack in all Node.js environments.

* 📝 Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants