-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat(eslint-config): typescript-eslint 8 #1326
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
160cbd0
to
8b3f003
Compare
8b3f003
to
bc2df9a
Compare
Size Change: 0 B Total Size: 64.2 kB
ℹ️ View Unchanged
|
CodSpeed Performance ReportMerging #1326 will create unknown performance changesComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1326 +/- ##
==========================================
- Coverage 76.73% 76.69% -0.05%
==========================================
Files 64 63 -1
Lines 503 502 -1
Branches 111 111
==========================================
- Hits 386 385 -1
Misses 111 111
Partials 6 6
|
@@ -13,6 +13,7 @@ const FALLBACK_GLOBAL = 'FALLBACK_GLOBAL' | |||
describe('<SuspensiveProvider/>', () => { | |||
it('should provide default ms prop of Delay', async () => { | |||
render( | |||
// eslint-disable-next-line @typescript-eslint/no-deprecated |
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.
suggestion)
This PR adds this eslint-disable-next-line
in several locations.
How about having it at 'warn' level so we can pass CI and no need to have so many eslint-disables..?
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.
While I agree that we are using eslint-disable-next-line excessively, I’m not sure if changing the rule to 'warn' is the best solution, as it could impact other files across the project.
For example, the most frequently disabled rule is @typescript-eslint/no-deprecated. This rule is mainly applied to deprecated components like Suspensive and DevMode, which are not accessible to users. These are very specific cases, and I’m concerned that if we lower the rule to 'warn', it could unintentionally affect other areas of the codebase where important rules like @typescript-eslint/no-deprecated should remain strictly enforced.
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.
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 file is intended to test components like Suspensive and SuspensiveProvider. Simply replacing them with DefaultPropsProvider, which doesn’t align with the original purpose, doesn’t seem like a good solution.
I believe that these deprecated files will likely be removed when Suspensive is upgraded to V3. Until then, if there isn’t a better alternative, how about temporarily suppressing the errors with eslint-disable-next-line?
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 thread is getting long, so to speed up the merge of this PR, I would appreciate it if you could first proceed in the way @SEOKKAMONI did, and then raise this issue later. I will create this as a separate issue. #1326
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.
Let's continue the discussion further in issue #1331! I’d like to express my gratitude to @saul-atomrigs for leaving a thoughtful comment that helps us dive deeper into this topic!
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.
@SEOKKAMONI thank you and great effort in this PR..!
Co-authored-by: Sol Lee <[email protected]> Co-authored-by: Varad Gupta <[email protected]> Co-authored-by: Jonghyeon Ko <[email protected]>
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.
❤️❤️❤️ Thanks!! ❤️❤️❤️
@@ -94,7 +94,7 @@ export const suspensiveTypeScriptConfig: ReturnType<typeof tseslint.config> = ts | |||
rules: vitest.configs.recommended.rules, | |||
settings: { vitest: { typecheck: true } }, | |||
}, | |||
jestDom.configs['flat/recommended'] as unknown as ReturnType<typeof tseslint.config>[number], |
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.
Beautiful..❤️
@@ -113,14 +113,14 @@ export const suspensiveTypeScriptConfig: ReturnType<typeof tseslint.config> = ts | |||
], | |||
}, | |||
}, | |||
eslintPluginPrettierRecommended as unknown as ReturnType<typeof tseslint.config>[number] |
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.
Happy..❤️
) | ||
|
||
export const suspensiveReactTypeScriptConfig: ReturnType<typeof tseslint.config> = tseslint.config( | ||
...suspensiveTypeScriptConfig, | ||
{ | ||
files: ['**/*.{ts,tsx}'], | ||
...(pluginReact.configs.recommended as unknown as ReturnType<typeof tseslint.config>[number]), |
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.
So cool...❤️
const returnOfJotai = useAtom(countAtom) | ||
expectTypeOf(result).toEqualTypeOf<typeof returnOfJotai>() | ||
return <></> | ||
}} | ||
</Atom> | ||
) | ||
;() => ( | ||
))() |
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.
Cool
@@ -1,60 +1,60 @@ | |||
import { queryOptions } from '@tanstack/react-query' |
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.
👍
Overview
related #1240
PR Checklist