-
Notifications
You must be signed in to change notification settings - Fork 189
chore: Upgrade eslint #3509
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: Upgrade eslint #3509
Conversation
01ea018
to
f54db28
Compare
@@ -15,7 +15,6 @@ const data1 = [...data, thresholdSeries]; | |||
// Position of the threshold series in a stacked chart must not affect chart's plot. | |||
const data2 = [data[0], thresholdSeries, data[1], data[2]]; | |||
|
|||
/* eslint-disable react/jsx-key */ |
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 now prints a warning if a disable directive does nothing. Removed all useless disables
@@ -130,7 +128,9 @@ export function DateForm({ filter, value, onChange }: PropertyFilterOperatorForm | |||
|
|||
// Parse value from filter text when it changes. | |||
useEffect(() => { | |||
filter && setState(parseDateTimeFilter(filter.trim())); |
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.
@typescript-eslint/no-unused-expressions
is enabled now and it fails on this pattern
@@ -8,7 +8,7 @@ import AppLayoutToolbar, { AppLayoutToolbarProps } from '../../../lib/components | |||
import createWrapper from '../../../lib/components/test-utils/dom'; | |||
import BreadcrumbGroup from '../../../lib/components/breadcrumb-group'; | |||
|
|||
export function renderComponent(jsx: React.ReactElement) { |
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.
jest/no-export
rule in action
src/app-layout/classic.tsx
Outdated
const stickyNotificationsHeight = stickyNotifications ? notificationsHeight ?? 0 : 0; | ||
const stickyNotificationsHeight = stickyNotifications ? (notificationsHeight ?? 0) : 0; |
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.
Prettier dependency got upgraded in this PR: 3.2.5 -> 3.5.3
@@ -1,5 +1,7 @@ | |||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
// SPDX-License-Identifier: Apache-2.0 | |||
|
|||
/* eslint-disable jest/no-export */ |
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 rule fails to detect test helper files. We do not have too many, disabled manually for now
@@ -278,7 +278,6 @@ export default function CollapsibleFlashbar({ items, ...restProps }: FlashbarPro | |||
> | |||
{showInnerContent(item) && ( | |||
<Flash | |||
// eslint-disable-next-line react/forbid-component-props |
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.
We do not have this rule enabled, react/forbid-component-props
960ab78
to
965e863
Compare
}, | ||
}, | ||
{ | ||
files: [ |
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.
Copied this list from .eslintrc.js
without changes. Related ticket: AWSUI-21112
filename: './src/button/index.tsx', | ||
options: [ruleHardcodedPath], | ||
errors: [{ message: "Disallowed import 'src/index.ts' in favor of having smaller bundle sizes." }], | ||
}, |
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.
Duplicated test. The new version detects and calls this out
@@ -1,13 +1,15 @@ | |||
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
// SPDX-License-Identifier: Apache-2.0 |
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.
Updated eslint captured all JS/TS files in our repo, not just src
@@ -65,6 +65,7 @@ function CornerMarker(props: { | |||
xmlns="http://www.w3.org/2000/svg" | |||
viewBox="0 0 10 10" | |||
style={{ inlineSize: '50px', blockSize: '50px', position: 'absolute', ...props }} | |||
focusable="false" |
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.
@cloudscape-design/components/no-implicit-svg-focusable
rule in action. Now across the whole project
@@ -18,7 +18,7 @@ describe('A11y checks for app layout toolbar', () => { | |||
.forEach(inputUrl => { | |||
const url = `#/light/${inputUrl}?${getUrlParams('refresh-toolbar')}`; | |||
test( | |||
url, | |||
`${url}`, |
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.
Fixing jest/valid-title
@@ -2,6 +2,7 @@ | |||
"compilerOptions": { | |||
"strict": true, | |||
"incremental": true, | |||
"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.
The default value is not an empty array but node_modules/@types/**
which loads some unnecessary typings in the scope
b951731
to
543e499
Compare
@@ -26,6 +26,7 @@ const defaultProps: DateRangePickerProps = { | |||
const outsideId = 'outside'; | |||
function renderDateRangePicker(props: DateRangePickerProps = defaultProps) { | |||
const ref = React.createRef<HTMLInputElement>(); | |||
console.log(props); |
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.
nit: small leftover
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.
fixed
210ecf2
to
451e5eb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3509 +/- ##
==========================================
- Coverage 96.62% 96.61% -0.01%
==========================================
Files 813 813
Lines 23687 23700 +13
Branches 8312 8305 -7
==========================================
+ Hits 22887 22898 +11
- Misses 746 795 +49
+ Partials 54 7 -47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Upgrade eslint to v9
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.