[extensions] false positive on external dependencies

[Zamiell]

Problem

The import-x/extensions rule has a false positive on external dependencies. (Furthermore, the autofixer incorrectly "fixes" the issues, which causes run-time errors.)

Details

First, turn on the import-x/extensions rule like this:

      "import-x/extensions": [
        "error",
        "always",
        {
          fix: true,
          pattern: {},
        },
      ],

(The reason for these strange options is because the autofixer is currently bugged, so ignore that for now.)

Once the rule is turned on, the rule will error on the following code:

import { foo } from "./foo";
import { GitVersionType } from "azure-devops-node-api/interfaces/GitInterfaces";

• Erroring on the first line is correct, because "foo.ts" exists in my code and import { foo } from "./foo.ts"; is a valid import.
• However, erroring on the second line is a bug, because "GitInterfaces.ts" does NOT exist. The autofixer will change it to import { GitVersionType } from "azure-devops-node-api/interfaces/GitInterfaces.ts";, which will cause a run-time error.

Thus, I propose that the rule should only error on non-external dependencies.

[JounQin]

Maybe we should not enable the autofix on relative paths?

[Zamiell]

Hi Joun, thanks for the quick reply.

Sorry, my issue description was worded confusingly.
The issue is not with the autofixer. The issue is that the rule itself marks the second line as an error.
Disabling the autofixer does not really help the situation, because then we would just have unfixable ESLint errors on import lines with external dependencies.

(More generally, continuing to have the autofixer work on relative paths is really important!)

[JounQin]

@Zamiell There is a ignorePackages option for this case, so try:

{
  "import-x/extensions": [
    "error",
    "ignorePackages",
    {
      "fix": true,
      "pattern": {} // will be unnecessary in next release
    }
  ]
}

[Zamiell]

Oh I'm really sorry, I did read the documentation before this but I misunderstood what the documentation was saying. 🤦

I propose that the ignorePackages option is changed to be the default in the next major version. Having always as the default option seems really dangerous, especially seeing as that I read in #235 we are already enabling the auto-fixer by default for this rule. Has there been any discussion of changing this recently?

[JounQin]

I propose that the ignorePackages option is changed to be the default in the next major version.

Yeah, I agree that ignorePackages should be the default. Another solution would be #395.

cc @SukkaW

[Zamiell]

I don't 100% understand what that PR is doing, but yes it would be very nice in principle if the extensions rule could somehow detect whether the changed import would actually result in a run-time error before flagging it.

[JounQin]

Yes, that PR ensures the fix won't change the resolved path.