ESLint plugin: @wordpress/dependency-group should have a way to integrate with aliases

[FerrielMelarpis]

Describe the bug When running wp-scripts lint-js, I get

error  Expected preceding "External dependencies" comment block  @wordpress/dependency-group

and the code that causes this is

import { someModule } from 'myalias/utils';
...

The problem is someModule is not an external dependency.

To reproduce Steps to reproduce the behavior:

1. Create a module
2. Add an alias to webpack.config.js pointing to that module
3. Import the module using the alias
4. Run wp-scripts lint-js

Expected behavior The error should be

error  Expected preceding "Internal dependencies" comment block  @wordpress/dependency-group

and when running --fix it should use Internal.

Desktop (please complete the following information):

• OS: MacOS Mojave 10.14.5
• Browser: Chrome
• Version 75.0.3770.142 (Official Build) (64-bit)

[swissspidy]

There's no way for the tool to just know that this is an alias on your system.

I see two possible solutions here:

1. You add // eslint-disable-line @wordpress/dependency-group to your code to disable the line there
2. We allow configuring the rule's assessment via your ESLint config file, e.g. by providing a regex or a list of package names that should be treated as internal.

[chrisvanpatten]

There's no way for the tool to just know that this is an alias on your system.

I expect this is not specifically an alias on @FerrielMelarpis's system, and rather an alias configured via the webpack "alias" option, e.g.

    resolve: {
        alias: {
            components: path.resolve( __dirname, '../app/block-editor/components' ),
        },
    },

I've noticed this particular issue as well, and do wonder if eslint could somehow parse the active webpack config and exclude any registered aliases from this rule.

The issue is obviously not hyper-critical, but it would be nice to resolve so the comments are as accurate as possible.

[swissspidy]

Sorry, yeah, that's what I meant. s/system/setup.

I think an ESLint rule should not need to be aware of Webpack configuration and stay independent.

Hence my suggestion to allow adjusting the rule's option.

For example, the complexity rule allows customization like this:

"complexity": ["error", { "max": 2 }]

For the @wordpress/dependency-group rule it could work as follows:

"@wordpress/dependency-group": ["error", { "isInternalDependency": "^\." }]

Where ^\. is the regex that is applied to the package name. ^\. is the default to match current behavior:

https://github.com/WordPress/gutenberg/blob/9fb2353092120957e88c4b35a4b1829eea3822f8/packages/eslint-plugin/rules/dependency-group.js#L52

IMHO that would be the proper way. If that makes sense I could try implementing it.

But anyways, I added the "Needs Technical Feedback" label to get more feedback on this 🙂

[FerrielMelarpis]

Thanks for the info @swissspidy I would love to have that kind of flexibility 👍 In the meantime, I just disabled that rule since it is not critical info in my code setup. I just find those comments helpful so I might use it again once it got support for custom regex rules.

[gziolo]

The @wordpress/dependency-group rule was removed from the recommended config somd time ago. See https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/CHANGELOG.md#300-2019-08-29 This should be no longer an issue.

[swissspidy]

@gziolo But the rule still exists and could be improved, no?

[gziolo]

But the rule still exists and could be improved, no?

Right, we can reopen and change the title 👍

[aaemnnosttv]

Would be great if this was extensible via the ESLint config as @swissspidy mentioned 🙏 . It might be a bit cleaner if this could be a single pattern OR a list of regex patterns to avoid a single highly-complex pattern for all. Then the plugin need only find if any of the provided patterns match.

patterns.find( ( pattern ) => ( new RegExp( pattern ) ).test( source ) );

Even though it is no longer part of the recommended ruleset, this causes a problem for projects that wish to use this rule and use aliases.

[swissspidy]

I've been working on this ESLint plugin recently. Might just as well fix this bug once and for all too 👍

Will give it a go.

[gziolo]

I've been working on this ESLint plugin recently. Might just as well fix this bug once and for all too 👍

Will give it a go.

I love your plan, thanks Pascal :)

[swissspidy]

@aaemnnosttv Please have a look at #20629. It's super simple, but perhaps already enough.

This rule has still some flaws and doesn't handle wildly unordered imports very well, but at least this helps with treating certain dependencies as internal ones.