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: add importNames support for patterns in no-restricted-imports #16059

Conversation

brandongregoryscott
Copy link
Contributor

@brandongregoryscott brandongregoryscott commented Jun 26, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[X] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Resolves #14274

What changes did you make? (Give an overview)

This change expands the functionality of no-restricted-imports to allow restricting only specific named imports from an import pattern ban. For example, if I wanted to ban importing Foo from '../../../my/relative/module', but importing Bar from the matched pattern is OK, I can now do that:

"no-restricted-imports": ["error", {            
    "patterns": [{
        "group": ["**/my/relative-module"],
        "importNames": ["Foo"],
        "message": "Don't import Foo from my/relative-module. Instead import from @repo/foo"
    }]
}]
import { Foo } from '../../../my/relative-module' // error
import { Foo, Bar } from '../../../my/relative-module' // error
import { Bar } from '../../../my/relative-module' // ok

Is there anything you'd like reviewers to focus on?

Nope - this is my first time contributing here, so please forgive me if I've missed anything - happy to update as needed!

@eslint-github-bot eslint-github-bot bot added the triage label Jun 26, 2022
@eslint-github-bot
Copy link

eslint-github-bot bot commented Jun 26, 2022

Hi @brandongregoryscott!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
  • There should be a space following the initial tag and colon, for example 'feat: Message'.

To Fix: You can fix this problem by running git commit --amend, editing your commit message, and then running git push -f to update this pull request.

Read more about contributing to ESLint here

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 26, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • login: brandongregoryscott / name: Brandon Scott (ba96189)

@netlify
Copy link

netlify bot commented Jun 26, 2022

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit fdd0dc1
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/62bdd6bdd99aee000837df59

@brandongregoryscott brandongregoryscott force-pushed the no-restricted-imports-pattern-with-import-names branch from fd6bf2d to f3e7849 Compare Jun 26, 2022
@eslint-github-bot
Copy link

eslint-github-bot bot commented Jun 26, 2022

Hi @brandongregoryscott!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

  • The length of the commit message must be less than or equal to 72

To Fix: You can fix this problem by running git commit --amend, editing your commit message, and then running git push -f to update this pull request.

Read more about contributing to ESLint here

@brandongregoryscott brandongregoryscott force-pushed the no-restricted-imports-pattern-with-import-names branch from f3e7849 to ba96189 Compare Jun 26, 2022
@eslint-github-bot eslint-github-bot bot added the feature label Jun 26, 2022
@brandongregoryscott brandongregoryscott changed the title feat(no-restricted-imports): add importNames functionality for patterns feat: add importNames support for restricted import patterns Jun 26, 2022
@mdjermanovic mdjermanovic added rule evaluating and removed triage labels Jun 27, 2022
@mdjermanovic mdjermanovic added enhancement accepted and removed evaluating labels Jun 28, 2022
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Thanks for working on this! I left two comments.

Also, this new feature should be documented in docs/src/rules/no-restricted-imports.md

lib/rules/no-restricted-imports.js Show resolved Hide resolved
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
@mdjermanovic mdjermanovic changed the title feat: add importNames support for restricted import patterns feat: add importNames support for patterns in no-restricted-imports Jun 29, 2022
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
docs/src/rules/no-restricted-imports.md Show resolved Hide resolved
@brandongregoryscott
Copy link
Contributor Author

brandongregoryscott commented Jun 30, 2022

I pushed up that change @mdjermanovic, feel free to take a look when you have time! I am noticing there's some duplicated logic in here, but I'm not sure if it's a concern right now.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Just a few small suggestions and then LGTM.

lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
tests/lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
tests/lib/rules/no-restricted-imports.js Show resolved Hide resolved
@mdjermanovic
Copy link
Member

mdjermanovic commented Jun 30, 2022

I am noticing there's some duplicated logic in here, but I'm not sure if it's a concern right now.

Yes, the code that handles patterns now looks very similar to the code that handles paths, but that's okay right now. At some point later we could consider refactoring.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

LGTM, thanks!

I'll leave this open if someone else wants to review it before merging.

btmills
btmills approved these changes Jul 2, 2022
Copy link
Member

@btmills btmills left a comment

LGTM. Thanks, and congrats on your first ESLint contribution, @brandongregoryscott! This will be going out in the release shortly.

@btmills btmills merged commit 7023628 into eslint:main Jul 2, 2022
20 checks passed
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Jul 4, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.18.0` -> `8.19.0`](https://renovatebot.com/diffs/npm/eslint/8.18.0/8.19.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.19.0`](https://github.com/eslint/eslint/releases/tag/v8.19.0)

[Compare Source](eslint/eslint@v8.18.0...v8.19.0)

#### Features

-   [`7023628`](eslint/eslint@7023628) feat: add importNames support for patterns in no-restricted-imports ([#&#8203;16059](eslint/eslint#16059)) (Brandon Scott)
-   [`472c368`](eslint/eslint@472c368) feat: fix handling of blockless `with` statements in indent rule ([#&#8203;16068](eslint/eslint#16068)) (Milos Djermanovic)

#### Bug Fixes

-   [`fc81848`](eslint/eslint@fc81848) fix: throw helpful exception when rule has wrong return type ([#&#8203;16075](eslint/eslint#16075)) (Bryan Mishkin)

#### Documentation

-   [`3ae0574`](eslint/eslint@3ae0574) docs: Remove duplicate rule descriptions ([#&#8203;16052](eslint/eslint#16052)) (Amaresh  S M)
-   [`f50cf43`](eslint/eslint@f50cf43) docs: Add base href to each page to fix relative URLs ([#&#8203;16046](eslint/eslint#16046)) (Nicholas C. Zakas)
-   [`ae4b449`](eslint/eslint@ae4b449) docs: make logo link clickable on small width screens ([#&#8203;16058](eslint/eslint#16058)) (Milos Djermanovic)
-   [`280f898`](eslint/eslint@280f898) docs: use only fenced code blocks ([#&#8203;16044](eslint/eslint#16044)) (Milos Djermanovic)
-   [`f5d63b9`](eslint/eslint@f5d63b9) docs: add listener only if element exists ([#&#8203;16045](eslint/eslint#16045)) (Amaresh  S M)
-   [`8b639cc`](eslint/eslint@8b639cc) docs: add missing migrating-to-8.0.0 in the user guide ([#&#8203;16048](eslint/eslint#16048)) (唯然)
-   [`b8e68c1`](eslint/eslint@b8e68c1) docs: Update release process ([#&#8203;16036](eslint/eslint#16036)) (Nicholas C. Zakas)
-   [`6d0cb11`](eslint/eslint@6d0cb11) docs: remove table of contents from markdown text ([#&#8203;15999](eslint/eslint#15999)) (Nitin Kumar)

#### Chores

-   [`e884933`](eslint/eslint@e884933) chore: use `github-slugger` for markdown anchors ([#&#8203;16067](eslint/eslint#16067)) (Strek)
-   [`02e9cb0`](eslint/eslint@02e9cb0) chore: revamp carbon ad style ([#&#8203;16078](eslint/eslint#16078)) (Amaresh  S M)
-   [`b6aee95`](eslint/eslint@b6aee95) chore: remove unwanted comments from rules markdown ([#&#8203;16054](eslint/eslint#16054)) (Strek)
-   [`6840940`](eslint/eslint@6840940) chore: correctly use .markdownlintignore in Makefile ([#&#8203;16060](eslint/eslint#16060)) (Bryan Mishkin)
-   [`48904fb`](eslint/eslint@48904fb) chore: add missing images ([#&#8203;16017](eslint/eslint#16017)) (Amaresh  S M)
-   [`910f741`](eslint/eslint@910f741) chore: add architecture to nav ([#&#8203;16039](eslint/eslint#16039)) (Strek)
-   [`9bb24c1`](eslint/eslint@9bb24c1) chore: add correct incorrect in all rules doc ([#&#8203;16021](eslint/eslint#16021)) (Deepshika S)
-   [`5a96af8`](eslint/eslint@5a96af8) chore: prepare versions data file ([#&#8203;16035](eslint/eslint#16035)) (Nicholas C. Zakas)
-   [`50afe6f`](eslint/eslint@50afe6f) chore: Included githubactions in the dependabot config ([#&#8203;15985](eslint/eslint#15985)) (Naveen)
-   [`473411e`](eslint/eslint@473411e) chore: add deploy workflow for playground ([#&#8203;16034](eslint/eslint#16034)) (Milos Djermanovic)
-   [`a30b66c`](eslint/eslint@a30b66c) chore: fix print style ([#&#8203;16025](eslint/eslint#16025)) (Amaresh  S M)
-   [`f4dad59`](eslint/eslint@f4dad59) chore: add noindex meta tag ([#&#8203;16016](eslint/eslint#16016)) (Milos Djermanovic)
-   [`db387a8`](eslint/eslint@db387a8) chore: fix sitemap ([#&#8203;16026](eslint/eslint#16026)) (Milos Djermanovic)
-   [`285fbc5`](eslint/eslint@285fbc5) chore: remove TOC from printable ([#&#8203;16020](eslint/eslint#16020)) (Strek)
-   [`8e84c21`](eslint/eslint@8e84c21) chore: remove ligatures from fonts ([#&#8203;16019](eslint/eslint#16019)) (Strek)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - 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 this update 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/1444
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted enhancement feature rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants