-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore: reorg repo level utils, lint and typecheck repo files #9618
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
Conversation
Thanks for the PR, @JamesHenry! 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. |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 32036ed. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 4 targetsSent with 💌 from NxCloud. |
8f505cd
to
78f6275
Compare
78f6275
to
4241c70
Compare
Co-Authored-By: Abraham Guo <[email protected]>
4241c70
to
c9881fe
Compare
Nice! Thanks a bunch for doing this and putting together the detailed walkthrough. I admire people who are knowledgeable and interested in the parts I'm not, like repo setup with |
@@ -2,7 +2,8 @@ | |||
"compilerOptions": { | |||
"types": ["@types/node"], | |||
"noEmit": true, | |||
"allowJs": true | |||
"allowJs": true, | |||
"allowImportingTsExtensions": true |
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.
why is this needed?
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.
Without it, yarn typecheck
's tsc
pass fails:
✖ nx run repo:typecheck
> tsc
tools/scripts/generate-configs.mts:18:8 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
18 } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-contributors.mts:10:27 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
10 import { REPO_ROOT } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-lib.mts:24:8 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
24 } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-sponsors.mts:6:34 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
6 import { PACKAGES_WEBSITE } from './paths.mts';
~~~~~~~~~~~~~
Found 4 errors in 4 files.
Errors Files
1 tools/scripts/generate-configs.mts:18
1 tools/scripts/generate-contributors.mts:10
1 tools/scripts/generate-lib.mts:24
1 tools/scripts/generate-sponsors.mts:6
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.
Yeah this was an option set in the tsconfig of repo-tools that I needed to carry over for this reason
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.
Looks great to me, thanks @JamesHenry! Excited to see one less CHANGELOG.md 😄
@@ -1,12 +1,17 @@ | |||
// @ts-check | |||
/* eslint-disable jsdoc/no-types */ |
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.
This makes me think we should change eslint.config.mjs
to only apply the TypeScript rules to TypeScript files. That's fine as a followup IMO.
packages/repo-tools/CHANGELOG.md
Outdated
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, good.
@@ -2,7 +2,8 @@ | |||
"compilerOptions": { | |||
"types": ["@types/node"], | |||
"noEmit": true, | |||
"allowJs": true | |||
"allowJs": true, | |||
"allowImportingTsExtensions": true |
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.
Without it, yarn typecheck
's tsc
pass fails:
✖ nx run repo:typecheck
> tsc
tools/scripts/generate-configs.mts:18:8 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
18 } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-contributors.mts:10:27 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
10 import { REPO_ROOT } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-lib.mts:24:8 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
24 } from './paths.mts';
~~~~~~~~~~~~~
tools/scripts/generate-sponsors.mts:6:34 - error TS5097: An import path can only end with a '.mts' extension when 'allowImportingTsExtensions' is enabled.
6 import { PACKAGES_WEBSITE } from './paths.mts';
~~~~~~~~~~~~~
Found 4 errors in 4 files.
Errors Files
1 tools/scripts/generate-configs.mts:18
1 tools/scripts/generate-contributors.mts:10
1 tools/scripts/generate-lib.mts:24
1 tools/scripts/generate-sponsors.mts:6
57a7c3f
Supersedes #8880
Explainer/walkthrough for this PR: https://www.loom.com/share/2c128b6caecb420abda03871f3043bbb