Skip to content
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-plugin): deprecate no-duplicate-imports in favour of import/no-duplicates #4973

Merged

Conversation

snewcomer
Copy link
Contributor

@snewcomer snewcomer commented May 14, 2022

PR Checklist

Overview

Deprecate no-duplicate-imports in favor of `import/no-duplicates.

Note: checking on a few codebases that this rule does what it needs for the TS community.

@nx-cloud
Copy link

nx-cloud bot commented May 14, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7824785. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 43 targets

Sent with 💌 from NxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @snewcomer!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@netlify
Copy link

netlify bot commented May 14, 2022

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 7824785
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/628288cb5f5f8000093bfc33
😎 Deploy Preview https://deploy-preview-4973--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bradzacher
Copy link
Member

There are two bits of config you can add to the rule metadata which will help signal the rule is deprecated. IIRC they are deprecated and deprecatedBy

@snewcomer snewcomer force-pushed the deprecate-no-duplicate-imports branch 2 times, most recently from b99ed62 to 5d0ef66 Compare May 15, 2022 16:39
@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #4973 (7116ffc) into main (2f57f3f) will increase coverage by 2.46%.
The diff coverage is n/a.

❗ Current head 7116ffc differs from pull request most recent head 7824785. Consider uploading reports for the commit 7824785 to get more accurate results

@@            Coverage Diff             @@
##             main    #4973      +/-   ##
==========================================
+ Coverage   91.78%   94.25%   +2.46%     
==========================================
  Files         229      154      -75     
  Lines       10627     8301    -2326     
  Branches     3289     2702     -587     
==========================================
- Hits         9754     7824    -1930     
+ Misses        591      263     -328     
+ Partials      282      214      -68     
Flag Coverage Δ
unittest 94.25% <ø> (+2.46%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/eslint-plugin/src/configs/all.ts 100.00% <ø> (ø)
...es/eslint-plugin/src/rules/no-duplicate-imports.ts 97.67% <ø> (ø)
packages/utils/src/ts-eslint/CLIEngine.ts
packages/utils/src/eslint-utils/index.ts
packages/utils/src/ts-eslint/Scope.ts
packages/utils/src/ts-eslint/ParserOptions.ts
packages/utils/src/eslint-utils/nullThrows.ts
packages/type-utils/src/requiresQuoting.ts
packages/utils/src/eslint-utils/RuleTester.ts
packages/typescript-estree/src/ts-estree/index.ts
... and 67 more

@snewcomer snewcomer force-pushed the deprecate-no-duplicate-imports branch from 5d0ef66 to 16e8266 Compare May 15, 2022 21:33
@snewcomer snewcomer marked this pull request as ready for review May 16, 2022 13:21
@snewcomer snewcomer force-pushed the deprecate-no-duplicate-imports branch from 16e8266 to 7116ffc Compare May 16, 2022 13:30
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

thanks for this!

@bradzacher bradzacher enabled auto-merge (squash) May 16, 2022 17:25
@bradzacher bradzacher changed the title chore: deprecate no-duplicate-imports chore(eslint-plugin): deprecate no-duplicate-imports May 16, 2022
@bradzacher bradzacher disabled auto-merge May 16, 2022 17:27
@bradzacher bradzacher changed the title chore(eslint-plugin): deprecate no-duplicate-imports feat(eslint-plugin): deprecate no-duplicate-imports in favour of import/no-duplicates May 16, 2022
@bradzacher bradzacher enabled auto-merge (squash) May 16, 2022 17:27
@bradzacher bradzacher merged commit 1d2e41a into typescript-eslint:main May 16, 2022
@@ -8,6 +8,8 @@ By default, TypeScript will type a catch clause variable as `any`, so explicitly

The `noImplicitAny` flag in TypeScript does not cover this for backwards compatibility reasons, however you can use `useUnknownInCatchVariables` (part of `strict`) instead of this rule.

## DEPRECATED
Copy link

Choose a reason for hiding this comment

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

Hi! I noticed that this has been deprecated, but this seems like an unrelated rule to no-duplicate-imports

Is this intentional? If not, is there a different rule we should use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added here

TS 4.4 now has a compiler flag for this. So it's now a rule we don't need! Because 4.4 is still in RC, we'll just deprecate it, and remove it in the next major.

Copy link

Choose a reason for hiding this comment

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

Oh, excellent! I definitely missed that. Thanks for the detail! :)

@snewcomer snewcomer deleted the deprecate-no-duplicate-imports branch May 17, 2022 17:44
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request May 19, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.23.0` -> `5.25.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.23.0/5.25.0) |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.23.0` -> `5.25.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.23.0/5.25.0) |

---

### Release Notes

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>

### [`v5.25.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5250-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5240v5250-2022-05-17)

[Compare Source](typescript-eslint/typescript-eslint@v5.24.0...v5.25.0)

##### Bug Fixes

-   **eslint-plugin:** \[typedef] stop enforcing rule for assignment expressions ([#&#8203;4958](typescript-eslint/typescript-eslint#4958)) ([04a216c](typescript-eslint/typescript-eslint@04a216c))
-   **eslint-plugin:** strict config should not extend recommended ([#&#8203;5005](typescript-eslint/typescript-eslint#5005)) ([05d71c2](typescript-eslint/typescript-eslint@05d71c2))
-   **website:** correct Presets link to be Configs ([#&#8203;5004](typescript-eslint/typescript-eslint#5004)) ([e18e91c](typescript-eslint/typescript-eslint@e18e91c))

##### Features

-   **eslint-plugin:** \[no-empty-function] new allow option overrideMethods ([#&#8203;4923](typescript-eslint/typescript-eslint#4923)) ([13c05ae](typescript-eslint/typescript-eslint@13c05ae))
-   **eslint-plugin:** deprecate `no-duplicate-imports` in favour of `import/no-duplicates` ([#&#8203;4973](typescript-eslint/typescript-eslint#4973)) ([1d2e41a](typescript-eslint/typescript-eslint@1d2e41a))
-   update to TypeScript 4.7-rc ([#&#8203;4829](typescript-eslint/typescript-eslint#4829)) ([6fe783c](typescript-eslint/typescript-eslint@6fe783c))

### [`v5.24.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5240-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5230v5240-2022-05-16)

[Compare Source](typescript-eslint/typescript-eslint@v5.23.0...v5.24.0)

##### Bug Fixes

-   **eslint-plugin:** \[array-type] in fixer add missing parens for constructor types [#&#8203;4756](typescript-eslint/typescript-eslint#4756) ([#&#8203;4971](typescript-eslint/typescript-eslint#4971)) ([0377070](typescript-eslint/typescript-eslint@0377070))

##### Features

-   **eslint-plugin:** Add BigInt object type to default ban-types list ([#&#8203;4970](typescript-eslint/typescript-eslint#4970)) ([1867728](typescript-eslint/typescript-eslint@1867728))
-   **eslint-plugin:** add new 'strict' config ([#&#8203;4706](typescript-eslint/typescript-eslint#4706)) ([4a500b2](typescript-eslint/typescript-eslint@4a500b2))

</details>

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>

### [`v5.25.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5250-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5240v5250-2022-05-17)

[Compare Source](typescript-eslint/typescript-eslint@v5.24.0...v5.25.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

### [`v5.24.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5240-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5230v5240-2022-05-16)

[Compare Source](typescript-eslint/typescript-eslint@v5.23.0...v5.24.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1357
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[no-duplicate-imports] deprecate rule in favour of import/no-duplicates
3 participants