pathGroupsExcludedImportTypes must be set to an empty array?

[thany]

Here's the relevant bit of our config:

    "import/order": [ "error", {
      "groups": [
        [ "builtin", "external" ],
        [ "internal" ],
        [ "parent", "sibling", "index" ],
        [ "object", "type" ]
      ],
      "pathGroups": [
        {
          "pattern": "Forms/**",
          "group": "internal",
          "position": "before"
        }
      ],
      "pathGroupsExcludedImportTypes": [],
      "newlines-between": "always"
    } ],

This works, I think. But I fail to understand why. The pathGroups, I assume, tells import/order the patterns that belong to internal imports.

But then why would I ever need to specify the position as well? Surely this is handled by groups already? 🤨

Also why does this only work when pathGroupsExcludedImportTypes is an empty array? I cannot understand for the life of me what that thing actually does. But it has to do with pathGroups, in some way or another, so I just started guessing at its value. The documentation is incredibly vague on this. Something is excluded - what is exlcuded, and from what? And what effect does it have on import order, and why?

Lastly, I shouldn't have to use pathGroups in the first place. This is a workaround for the fact that even with a parser that understands jsconfig.json or tsconfigjson, after 2 years it still doesn't see that these aliased paths actually belong to internal. I haven't looked at the code, but it feels like import/order way of deterining what sort of import is external vs internal, is hardcoded, instead of taken from any kind of configuration to speak of.

I'd like to keep my configs DRY, but that seems impossible with this plugin. Can we get this fixed please?

Recap:

• position shouldn't be neccesary - handled by groups order.
• pathGroupsExcludedImportTypes is vague:
  • Needs better explanation
  • Might need a more sane default value
  • Should not be neccesary when just using pathGroups to "assign" certain paths to a group
• Please respect project config in jsconfig.json or tsconfig.json. Using one of these is the defacto standard to configure path aliases. And ESLint is aware of these when using a parser that requires them.

[ljharb]

It would help if you made a PR with failing test cases.

I don't see why we'd want to respect jsconfig.json, which isn't really a thing, but we do respect tsconfig.json already afaik.

It makes sense that pathGroupsExcludedImportTypes shouldn't need to be specified at all; a PR for that would be appreciated also.

[thany]

I don't see why we'd want to respect jsconfig.json, which isn't really a thing, but we do respect tsconfig.json already afaik.

Ok then that's a bug. Because neither work. And jsconfig.json a very much a thing, although maybe you're not using it currently.

It makes sense that pathGroupsExcludedImportTypes shouldn't need to be specified at all; a PR for that would be appreciated also.

I would think it's simply a matter of setting the default value to [], is it not?

[ljharb]

Yes, I’d think so.

[ysdevaestro]

I have the same issue, and I had to add an empty array for pathGroupsExcludedImportTypes to make it work.
Thank you @thany for this hack :)

[thumbsupep]

The documentation specifies that the default value for pathGroupsExcludedImportTypes is ["builtin", "external"] so I dont think we can just make a PR to change that to [] without releasing a new major version.

also, in the code it looks like the default is actually ["builtin", "external", "object"]

what was the reason for using ["builtin", "external", "object"] as the default?

[thany]

The documentation specifies that the default value for pathGroupsExcludedImportTypes is ["builtin", "external"] so I dont think we can just make a PR to change that to [] without releasing a new major version.

It depends. To whom does it make sense to have anything in that array? Because it's so vague as to what it's for, I seriously doubt many people will be needing it.

also, in the code it looks like the default is actually ["builtin", "external", "object"]

what was the reason for using ["builtin", "external", "object"] as the default?

What was the reason for having this property at all? 😉