Skip to content

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

chore: Upgrade eslint #3509

wants to merge 2 commits into from

Conversation

just-boris
Copy link
Member

Description

Upgrade eslint to v9

Related links, issue #, if available: n/a

How has this been tested?

  • PR build
  • Made some rule violations locally, saw eslint errors
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -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 */
Copy link
Member Author

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()));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tuned @typescript-eslint/no-unused-expressions rule to forbid 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) {
Copy link
Member Author

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

const stickyNotificationsHeight = stickyNotifications ? notificationsHeight ?? 0 : 0;
const stickyNotificationsHeight = stickyNotifications ? (notificationsHeight ?? 0) : 0;
Copy link
Member Author

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 */
Copy link
Member Author

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant