Eslint-plugin-import: pathGroups using minimatch instead of real regexes

[ksjogo]

The new pathGroup addition is using minimatch, this being based on shell glob expansions makes it incredible hard to do differing matching on paths containing ../ updots, as these are not easily (if at all) added to the minimatch expression in unspecified depth.
Yes, the codebase should import from the roots and not jump up arbitrarily, but sometimes the world is at it is.
This shortcoming is not evident inside the PR tests themselves, as all these prefix with special characters: https://github.com/benmosher/eslint-plugin-import/pull/1386/files
I didn't find a way to express my wanted regex with minimatch, it would be great to have a shortcutting flag to just supply a real regex.

[ljharb]

I find regexes in config files to be a massive antipattern, and am not interested in adding that.

Can you give me a concrete example of a pathGroups setting you're trying, and the code you want it to work on?

[ksjogo]

Yes, the codebase should indeed use proper root based imports and no relative ones, but that isn't necessarily the case.
Our current configuration:

"import/order": [
            "error",
            {
                "newlines-between": "always",
                "alphabetize": {
                    "order": "asc",
                    "caseInsensitive": true
                },
                "groups": [
                    "builtin",
                    "external",
                    "internal",
                    "parent",
                    "sibling",
                    "index"
                ],
                "pathGroups": [
                    {
                        "regex": "(..\/)*(actions|graphql|hocs|sagas)\/?.*",
                        "group": "internal"
                    },
                    {
                        "regex": "[.]\/components|[.]\/containers|components\/SVG",
                        "group": "internal",
                        "position": "after"
                    },
                    {
                        "regex": "([.]{2}\/[A-Z][a-zA-Z]+)|(^[.]{2}\/SVG)",
                        "group": "parent",
                        "position": "before"
                    },
                    {
                        "regex": "[.]{2}\/[a-z][a-zA-Z.\/]+$",
                        "group": "parent",
                        "position": "after"
                    }
                ]
            }
        ],

Which runs perfectly fine with my fork with proper regexes.
These seem to be unable to be approximated by minimatch because minimatch is doing hard splits on every / as globs are built to match to the specific directory structure given, whereas in this case the amounts of ../ will actually vary between files and require different grouping.
I have a fork on my work computer, which I will upload tomorrow. Basically just adding a regex to the config options, making the schema oneOf either and then just constructing a regex object and matching that.

[ljharb]

I'm not interested in a regex approach; I'd rather fix the way we're using minimatch.

In other words, I'd expect you to be able to use globs - multiple globs in an array if possible - to achieve what you want, and if you can't right now, let's make that happen.

[ksjogo]

What's the reason for not wanting to support regex?
They are better known then globs usually and minimatch is creating these anyway (actually more complex ones then I posted here).

[ljharb]

Supporting regexes in eslint configs leads to all sorts of CVE vulnerabilities about catastrophic backtracking, for one - but also I think it's overpowered, and it'd be better to explicitly support the needed features.

As for "better known", globs are what eslint itself uses, as well as gitignore and npmignore, so I'm not sure that's true.

[ksjogo]

Okay, didn't think about redos here. Was there some concrete example for this in the past, I am unfamiliar with eslints internal happenings and in which case an attacker would be able to inject things into my eslint config (apart from all the code usually pulled unseen from npm anyway, which could do whatever anyway?).

I pretty much remember my university modules about regex and FSMs, never had any deeper looks into globs. :D

I suppose we could add some way to strip of suffixes from the name via config before minimatching and achieve the same.

[ljharb]

Yes, there was a plethora of CVEs filed on eslint plugins a year or three ago for exactly this.

[tulir]

I also want to match an arbitrary number of parent directories.

I have a web app that imports external dependencies from /web_modules/ (using snowpack). However, I want it to be able to run on arbitrary paths rather than requiring dependencies to be at the root of the domain or on a specific subpath, so even the external dependency imports are relative.

I'm currently using {.,..,../..,../../..}/web_modules/** as the pattern. It's very ugly and doesn't support arbitrary amounts of nesting, but works fine for now.

[ljharb]

@tulir **/web_modules/** doesn't work for that?

[tulir]

Pretty sure it doesn't:

> minimatch("../../web_modules/preact.js", "**/web_modules/**")
false
> minimatch("../../web_modules/preact.js", "{.,..,../..,../../..}/web_modules/**")
true

[ljharb]

I'm not too convinced by snowpack as a use case, since this plugin relies on node_modules living on disk, and everything coming from npm install.

However, this seems like it would be better solved by using a snowpack resolver instead of the node resolver + a bunch of rule configuration overrides.

[ksjogo]

Would you be okay with adding an option that strips of an arbitrary amount of ../ before matching?

[ljharb]

That still seems like a hacky workaround for not using a snowpack resolver, if you're using snowpack.

[Menci]

minimatch is buggy -- isaacs/minimatch#30 which stops path like ./aaa, ../aaa be matched together with @internal/aaa by **.

[ljharb]

That doesn't seem like a bug to me. ** doesn't grab dotfiles.

[Menci]

So how on the earth can I match both @/aaa/bbb/ccc/foobar and ./foobar with minimatch?

[ksjogo]

You can use my fork which allows proper reg exs.

[ljharb]

@Menci ['**/foobar', '**/.*/foobar']?

[Menci]

@ljharb Thanks. I didn't see in the docs that the pattern can be array.