-
Notifications
You must be signed in to change notification settings - Fork 101
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
update ESLint to v9 and other smaller dependencies #1635
Conversation
|
78fb56d
to
9355a95
Compare
@@ -0,0 +1,216 @@ | |||
/* |
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 was the bulk of the work, please read this file
@@ -483,7 +483,7 @@ class SourceCodeEditor extends Component<SourceCodeEditorProps> { | |||
const { styles } = this.props | |||
|
|||
if (!styles?.theme || !styles.highlightStyle) { | |||
return | |||
return undefined |
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 here?
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.
ESLint expects that a getter always returns a value. IMO this is a good rule, and its OK if we make such edge cases more explicit
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.
Just a thought: shouldn't we turn "Unexpected any. Specify a different type" warnings off? Those are polluting the log and serve no real info.
Otherwise: great work!
Yep, it gets a bit spammy because we have a lot of these. But I think they are useful, we should try to get rid of |
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 work
…x warnings in Table ESLint now uses a flat config. our ESLint plugin is not in use yet, it will be added in a future commit
Closes: INSTUI-4099, INSTUI-4062, INSTUI-4183
to test: check docs app, check output of
npm run lint
for erroneous lint warnings.The good:
ui-eslint-config
package is gone :)packages/template-
packages had some leftover files that could be deleted.eslintignore
filesvitest
testsThe bad:
eslint-plugin-jsx-a11y
is not yet converted to ESLint v9, add it back when its ready Add support for ESLint 9 jsx-eslint/eslint-plugin-jsx-a11y#978eslint-import-resolver-typescript
andeslint-plugin-import-x
are not converted either, but these are less useful pluginsOther smaller changes:
isStacked
andhover
prop that was always showing in the consolemoment
upgrade, turns out the test was bad and moment got better