-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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: use recommended config for Node.js 18 in TSConfig #9113
Conversation
Thanks for the PR, @lesha1201! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -2,7 +2,6 @@ | |||
"extends": "./tsconfig.build.json", | |||
"compilerOptions": { | |||
"composite": false, | |||
"target": "ES2022", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It extends tsconfig.base.json
so no need to specify it anymore.
@@ -2,7 +2,6 @@ | |||
"extends": "./tsconfig.json", | |||
"compilerOptions": { | |||
"outDir": "../../dist/out-tsc", | |||
"module": "Node16", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It extends tsconfig.base.json
so no need to specify it.
Note: it looks like #9038 already contains some changes in this area |
@@ -6,7 +6,7 @@ | |||
"declaration": true, | |||
"declarationMap": true, | |||
"esModuleInterop": true, | |||
"lib": ["ES2021"], | |||
"lib": ["ES2023"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to ask why ES2023 was legal, but node.green does in fact show that all versions of Node 18 support all runtime features of ES2023. I've fixed https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping to reflect this (which matches the bases too).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!!
@@ -16,6 +16,6 @@ | |||
"skipLibCheck": true, | |||
"sourceMap": true, | |||
"strict": true, | |||
"target": "ES2021" | |||
"target": "ES2022" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: I'm surprised TS 5.4 doesn't support an ES2023 target. I tried that locally and got:
✖ nx run ast-spec:build
../../tsconfig.base.json(19,15): error TS6046: Argument for '--target' option must be: 'es5', 'es6', 'es2015', 'es2016', 'es2017', 'es2018', 'es2019', 'es2020', 'es2021', 'es2022', 'esnext'.
Warning: command "yarn build" exited with non-zero status code
microsoft/TypeScript#56303 - turns out es2023 === es2022 for target so it was kept out. Huh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was rectified in microsoft/TypeScript#58140 and we plan to not skip again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super, thanks! 🚀
PR Checklist
Overview
Changed TSConfig to target Node.js 18 since this is the minimal version we support. According to https://node.green/, Node.js 18 fully supports ES2022 and ES2023:
There is a recommended TSConfig for Node.js 18 - https://github.com/tsconfig/bases/blob/main/bases/node18.json and I followed it as a reference.
In another PR, I'm planning to enable ESLint https://eslint.org/docs/latest/rules/prefer-object-has-own rule and fix all related errors.