Skip to content

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented Jun 23, 2024

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:

What changes did you make? (Give an overview)

Fix #18536

/*eslint no-restricted-imports: ["error", { patterns: [{
    "importNamePattern": "_Enum$",
    "regexGroup": "@app/(?!(api/enums$)).*"
}]}]*/

// error
import { Foo_Enum } from '@app/api';
import { Bar_Enum } from '@app/api/bar';
import { Baz_Enum } from '@app/api/baz';
import { B_Enum } from '@app/api/enums/foo';

// no error
import { C_Enum } from '@app/api/enums';

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

Earlier group was a required property in the schema and would throw an error if not defined, which isn't the case anymore as we need either group or regexGroup.

@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Jun 23, 2024
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Jun 23, 2024
Copy link

netlify bot commented Jun 23, 2024

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 0f8ef13
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/668126fbc5e2f2000817328f
😎 Deploy Preview https://deploy-preview-18622--docs-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 configuration.

@snitin315 snitin315 force-pushed the fix/regexp-group-opt branch from 70ca81f to 1c65ff0 Compare June 23, 2024 11:25
@snitin315 snitin315 marked this pull request as ready for review June 23, 2024 11:39
@snitin315 snitin315 requested a review from a team as a code owner June 23, 2024 11:39
@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jun 23, 2024
@aladdin-add aladdin-add changed the title feat: add regexGroup option in no-restricted-imports feat: add regex option in no-restricted-imports Jun 24, 2024
not: {
anyOf: [
{ required: ["group", "regex"] },
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{ required: ["group", "regex"] },

I think this restriction is now already covered by the oneOf below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, updated.

aladdin-add
aladdin-add previously approved these changes Jun 25, 2024
Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM. Would like @mdjermanovic to verify before merging.

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@mdjermanovic
Copy link
Member

I think the documentation for patterns needs more updates because, throughout the ### patterns section, it was assumed that it's always based on .gitignore-style patterns and that group property must be specified.

For example:

This is also an object option whose value is an array. This option allows you to specify multiple modules to restrict using `gitignore`-style patterns.

Because the patterns follow the `gitignore`-style, if you want to reinclude any particular module this can be done by prefixing a negation (`!`) mark in front of the pattern. (Negated patterns should come last in the array because order is important.)

This is a boolean option and sets the patterns specified in the `group` array to be case-sensitive when `true`. Default is `false`.

You can also specify `importNames` on objects inside of `patterns`. In this case, the specified names are applied only to the specified `group`.

You can also specify `allowImportNames` on objects inside of `patterns`. In this case, the specified names are applied only to the specified `group`.

@snitin315
Copy link
Contributor Author

@mdjermanovic Updated.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Dec 31, 2024
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rule Change: Support simple regex in no-restricted-imports group
3 participants