-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: run eslint with type information on CI #13368
Conversation
@@ -363,19 +362,16 @@ const makeThrowingMatcher = ( | |||
})(); | |||
|
|||
if (isPromise(potentialResult)) { | |||
const asyncResult = potentialResult as AsyncExpectationResult; |
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.
only change to non-test code. The cast was unnecessary as TS could infer it from it being a promise
@@ -114,10 +114,10 @@ const IDVisitor = { | |||
], | |||
}; | |||
|
|||
const FUNCTIONS: Record< | |||
const FUNCTIONS = Object.create(null) as Record< |
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.
Object.create
returns any
'babel-jest', | ||
'babel-plugin-jest-hoist', | ||
'diff-sequences', | ||
'jest-source-map', |
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.
Only four packages? That’s not su bad. It is indeed good idea to have this added!
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 is a list of which we lint, not the ones excluded. running on e.g. expect
gives almost 400 errors, so I stopped there 😅
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.
Ah.. So that’s a good start then ;D
loadPartialConfig: jest.fn((...args) => actual.loadPartialConfig(...args)), | ||
loadPartialConfigAsync: jest.fn((...args) => | ||
actual.loadPartialConfigAsync(...args), | ||
loadPartialConfig: jest.fn<typeof actual.loadPartialConfig>((...args) => |
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.
#13367 came from this 🙂
Current state running against all packages btw: |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
We OOM if we run the entire lint run with type info (see https://typescript-eslint.io/docs/linting/typed-linting/monorepos/#important-note-regarding-large--10-multi-package-monorepos), so I've added a script.
We have a toooooon of errors, so I just fixed a few of them. Should chip away at this when people have time 🙂
This is somewhat annoying as IDE integrations etc won't apply, but better than nothing
Test plan
Added CI run