refactor: update the default behavior of import check, closes #858

[Rel1cx]

What kind of change does this PR introduce?

• Bugfix
• Feature
• Docs
• New Binding issue #___
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change?

• Yes, and the changes were approved in issue #___
• No

Checklist

• When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
• I have added a convincing reason for adding this feature, if necessary

Other information

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eslint-react	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Nov 19, 2024 0:37am

[kachkaev]

👋 @Rel1cx! I think I’ve noticed new issue in @eslint-react/no-array-index-key, and bisecting has pointed me to this commit.

In 1.16.2-beta.6 code like this produces a linting error, which is expected:

foo.bar.map((value, index) => <MyComponent key={index} />);

However, after upgrading to 1.16.2-beta.7, the issue is no longer reported.

If I replace property chaining with const { bar } = foo; and then call bar.map(...) in JSX, @eslint-react/no-array-index-key is reported again as expected.

I am thinking of creating a new issue but in needs a proper reproduction and I don’t have time on it today. I’ve decided to ping you here first with my initial investigation to just sense-check what I observe. WDYT?

[Rel1cx]

👋 @Rel1cx! I think I’ve noticed new issue in @eslint-react/no-array-index-key, and bisecting has pointed me to this commit.

In 1.16.2-beta.6 code like this produces a linting error, which is expected:

foo.bar.map((value, index) => <MyComponent key={index} />);

However, after upgrading to 1.16.2-beta.7, the issue is no longer reported.

If I replace property chaining with const { bar } = foo; and then call bar.map(...) in JSX, @eslint-react/no-array-index-key is reported again as expected.

I am thinking of creating a new issue but in needs a proper reproduction and I don’t have time on it today. I’ve decided to ping you here first with my initial investigation to just sense-check what I observe. WDYT?

Hello! I just reviewed the code and have pinpointed the issue and started fixing it. Your initial investigation was correct and helpful; this commit indeed introduced the problem. Thank you for providing the information.

[kachkaev]

Great, thanks for responding so quickly @Rel1cx! I just tried 1.17.3-beta.1 (which included #868) and it worked 🎉

[kachkaev]

Another thing that I noticed after upgrading from 1.16.1 to newer versions is @eslint-react/prefer-read-only-props triggering everywhere. We use this coding style for React components:

import * as React from 'react';

export const Foo: React.FunctionComponent<{
  bar: string;
}> = ({ bar }) => {
  return <>{bar}</>;
};

The rule is included in recommended-type-checked but it was not triggering for us, probably until this PR.

I believe that we have a bug fix rather than a new problem, so no action needed on this one. Just sharing my observation with you for context. You might want to add some new tests though to avoid future regressions.