chore(site): align ESLint config to typescript-eslint's recommended-requiring-type-checking #5797
Conversation
…equiring-type-checking
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
Wait wait, it's still a draft and I still have to fix or disable the lint complaints. There's much more to come 😄 |
|
I have read the CLA Document and I hereby sign the CLA |
|
@JoshuaKGoldberg I figured, but still agree with the thread linked. I'll re-review once all the changes are made! |
| @@ -20,8 +20,7 @@ export const hardCodedCSRFCookie = (): string => { | |||
| export const withDefaultFeatures = ( | |||
| fs: Partial<TypesGen.Entitlements["features"]>, | |||
| ): TypesGen.Entitlements["features"] => { | |||
| for (const k in TypesGen.FeatureNames) { | |||
| const feature = k as TypesGen.FeatureName | |||
| for (const feature of TypesGen.FeatureNames) { | |||
There was a problem hiding this comment.
https://typescript-eslint.io/rules/no-for-in-array
A for-in loop (
for (var i in o)) iterates over the properties of an Object. While it is legal to use for-in loops with array types, it is not common. for-in will iterate over the indices of the array as strings, omitting any "holes" in the array.
| @@ -152,8 +151,7 @@ export const getTokens = async (): Promise<TypesGen.APIKey[]> => { | |||
| } | |||
|
|
|||
| export const deleteAPIKey = async (keyId: string): Promise<void> => { | |||
| const response = await axios.delete("/api/v2/users/me/keys/" + keyId) | |||
| return response.data | |||
| await axios.delete("/api/v2/users/me/keys/" + keyId) | |||
There was a problem hiding this comment.
This function indicates it returns Promise<void>, so any returned data is ignored. It's violating @typescript-eslint/no-unsafe-return anyway so I figured I'd clean it up. 🙂
| const bannerPillSingular = await screen.queryByText(Language.licenseIssue) | ||
| const bannerPillPlural = await screen.queryByText(Language.licenseIssues(2)) | ||
| const bannerPillSingular = screen.queryByText(Language.licenseIssue) | ||
| const bannerPillPlural = screen.queryByText(Language.licenseIssues(2)) |
There was a problem hiding this comment.
screen.queryByText, screen.getByRole, etc. don't return Promises. There's no need to await them.
| await ( | ||
| await getUsers(queryToFilter(query)) | ||
| ).users, | ||
| (await getUsers(queryToFilter(query))).users, |
There was a problem hiding this comment.
This also is an await of something (.users) that is not async.
|
@JoshuaKGoldberg thx for your contribution! There is just one minor conflict to solve before we merge it. Unfortunately, I don't have permission for it. |
|
...huh, I swear I used to know where that option was. Fixed! |
From https://twitter.com/jsjoeio/status/1616168014172557314:
@typescript-eslint/no-floating-promises)@typescript-eslint/camelcase)I'd definitely recommend filing those TODOs in your backlog and linking to them in the config file. Most of those rules can catch an instance of
anyor some other unfortunate practice sneaking into your app. Though, note that the ones around template string concatenation might not have as many (or any) reports after typescript-eslint/typescript-eslint#6014 lands.