[New] `no-internal-modules`: Add `forbid` option #1846

guillaumewuip · 2020-07-04T19:15:15Z

Here is the pull request that implements #1842.

Without forbid option provided, the behavior stay the same. It should be a semver-minor PR.

With forbid option provided, the rule check if a given path is explicitly forbidden before testing if it's allowed.

guillaumewuip · 2020-07-04T19:16:13Z

tests/src/rules/no-internal-modules.js

@@ -14,6 +14,13 @@ ruleTester.run('no-internal-modules', rule, {
      filename: testFilePath('./internal-modules/plugins/plugin.js'),


I've added tests, but maybe not enough. Don't hesitate to ask me to implement other tests if you see something I've missed :)

coveralls · 2020-07-04T19:40:55Z

Coverage increased (+0.009%) to 97.879% when pulling 1c6a7ca on guillaumewuip:add-forbid-option-no-internal-modules into fd4b16b on benmosher:master.

coveralls · 2020-07-04T19:40:55Z

Coverage decreased (-0.006%) to 97.851% when pulling 345cfb0 on guillaumewuip:add-forbid-option-no-internal-modules into 843055c on benmosher:master.

ljharb

I'd saw this is worth a lot more test cases :-) I don't have any concrete ones in mind, but the more there are, the more convincing the PR is.

ljharb · 2020-07-06T05:10:53Z

docs/rules/no-internal-modules.md

@@ -4,7 +4,10 @@ Use this rule to prevent importing the submodules of other modules.

 ## Rule Details

-This rule has one option, `allow` which is an array of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns that whitelist paths and import statements that can be imported with reaching.
+This rule has two options that are arrays of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns:


Suggested change

This rule has two options that are arrays of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns:

This rule has two mutually exclusive options that are arrays of [minimatch/glob](https://github.com/isaacs/node-glob#glob-primer) patterns:

ljharb · 2020-07-06T05:12:21Z

docs/rules/no-internal-modules.md

+      "allow": [ "**/actions/*", "source-map-support/*" ],
+      "forbid": [ "**/actions/special/*" ],


should it be possible to both allow and forbid specific things? When "allow" is specified, it takes precedence over a default "forbid" of "/*". When "forbid" is specified, it takes precedence over a default "allow" of "/*".

I'd expect the schema to only allow one or the other, never both.

By

I'd expect the schema to only allow one or the other, never both.

You mean the user will either provide allow array or forbid array but never both ?

Correct. If both are provided, we have to define a precedence between them - with allow of A and forbid of B, what's intended with C? Is it allowed, because it's not forbidden? Or is it forbidden, because it's not allowed?

Yep. I understand now! I will rework the PR to do that 👍

Fixes #1842

guillaumewuip · 2020-07-17T15:59:53Z

@ljharb I took the time to work again on this PR. allow and forbid are now mutually exclusive. Same default behavior, should not be a breaking

guillaumewuip · 2020-10-05T06:18:34Z

Hello @ljharb, do you need something else on this PR ?

jenskuhrjorgensen · 2021-02-10T10:36:13Z

Hi @ljharb

This feature seems to have been released in the docs, but it is not released in the newest release (v.2.22.1) which is pretty confusing. Is this a mistake? I was trying to use the feature as described in the docs, but it failed with the following error:

Error: .eslintrc:
	Configuration for rule "import/no-internal-modules" is invalid:
	Value {"forbid":["**/actions/*","source-map-support/*"]} should NOT have additional properties.

Best regards
Jens

ljharb · 2021-02-10T16:31:47Z

It's not a mistake at all. If you're looking at the docs on master, you'll see unreleased things.

I created the "github release" for v2.22.1 yesterday, but it was actually released in September (see the changelog link). This PR has not yet been released.

jenskuhrjorgensen · 2021-02-11T07:26:50Z

Okay, thanks for clarifying. Never realized that I actually have to look in the release zip for the current documentation :)

ljharb · 2021-02-11T14:58:31Z

Yes, on every project on github, you have to switch to the latest release tag to see the current documentation.

richard-lopes-ifood · 2021-03-05T23:27:07Z

When does it goes to an official release?

guillaumewuip commented Jul 4, 2020

View reviewed changes

ljharb requested changes Jul 6, 2020

View reviewed changes

ljharb added the semver-minor label Jul 6, 2020

[New] no-internal-modules: Add forbid option

1c6a7ca

Fixes #1842

ljharb changed the title ~~[New] Add forbid options for no-internal-modules~~ [New] no-internal-modules: Add forbid option Jan 30, 2021

ljharb approved these changes Jan 30, 2021

View reviewed changes

ljharb merged commit 1c6a7ca into import-js:master Jan 31, 2021

guillaumewuip deleted the add-forbid-option-no-internal-modules branch January 31, 2021 12:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[New] `no-internal-modules`: Add `forbid` option #1846

[New] `no-internal-modules`: Add `forbid` option #1846

guillaumewuip commented Jul 4, 2020

guillaumewuip Jul 4, 2020

coveralls commented Jul 4, 2020 •

edited

coveralls commented Jul 4, 2020

ljharb left a comment

ljharb Jul 6, 2020

ljharb Jul 6, 2020

guillaumewuip Jul 6, 2020

ljharb Jul 6, 2020

guillaumewuip Jul 6, 2020

guillaumewuip commented Jul 17, 2020

guillaumewuip commented Oct 5, 2020

jenskuhrjorgensen commented Feb 10, 2021

ljharb commented Feb 10, 2021

jenskuhrjorgensen commented Feb 11, 2021

ljharb commented Feb 11, 2021

richard-lopes-ifood commented Mar 5, 2021 •

edited

		@@ -14,6 +14,13 @@ ruleTester.run('no-internal-modules', rule, {
		filename: testFilePath('./internal-modules/plugins/plugin.js'),

	This rule has two options that are arrays of [minimatch/glob patterns](https://github.com/isaacs/node-glob#glob-primer) patterns:
	This rule has two mutually exclusive options that are arrays of [minimatch/glob](https://github.com/isaacs/node-glob#glob-primer) patterns:

		"allow": [ "*/actions/", "source-map-support/*" ],
		"forbid": [ "*/actions/special/" ],

[New] no-internal-modules: Add forbid option #1846

[New] no-internal-modules: Add forbid option #1846

Conversation

guillaumewuip commented Jul 4, 2020

guillaumewuip Jul 4, 2020

Choose a reason for hiding this comment

coveralls commented Jul 4, 2020 • edited

coveralls commented Jul 4, 2020

ljharb left a comment

Choose a reason for hiding this comment

ljharb Jul 6, 2020

Choose a reason for hiding this comment

ljharb Jul 6, 2020

Choose a reason for hiding this comment

guillaumewuip Jul 6, 2020

Choose a reason for hiding this comment

ljharb Jul 6, 2020

Choose a reason for hiding this comment

guillaumewuip Jul 6, 2020

Choose a reason for hiding this comment

guillaumewuip commented Jul 17, 2020

guillaumewuip commented Oct 5, 2020

jenskuhrjorgensen commented Feb 10, 2021

ljharb commented Feb 10, 2021

jenskuhrjorgensen commented Feb 11, 2021

ljharb commented Feb 11, 2021

richard-lopes-ifood commented Mar 5, 2021 • edited

[New] `no-internal-modules`: Add `forbid` option #1846

[New] `no-internal-modules`: Add `forbid` option #1846

coveralls commented Jul 4, 2020 •

edited

richard-lopes-ifood commented Mar 5, 2021 •

edited