Make use of typescript import resolver

[mrhyde]

This pull request includes eslint-import-resolver-typescript as a peer dependency and enables TypeScript resolution for eslint-plugin-import. And sets import/no-unresolved to error

Doing so we will also avoid having extraneous looking dependency in consumers package.json:

...
    "eslint-config-love": "46.0.0",
    "eslint-import-resolver-typescript": "3.6.1",
...

[mrhyde]

Hi! Is there anything I can do to help get this feature merged and a new version published?

[mightyiam]

All efforts are currently towards flat config.

You could apply for joining the work sessions.

[mrhyde]

I couldn't find recent info on the sessions you mentioned. How do I apply?

[mightyiam]

It should be renamed but here it is: https://mobusoperandi.com/mobs/standard.html

[mightyiam]

For reference:

https://chatgpt.com/share/e2a0b615-d355-4152-b856-cb251152ceb3

[mightyiam]

I'm sorry for breaking your PR, @mrhyde . I'd ask you to rebase, but I haven't yet acquired an understanding. Sorry for taking long.

[mrhyde]

Done :)

[mightyiam]

Thank you, @mrhyde.

I'm looking into this and after some understanding I find myself wanting to see a reproduction of the problem this solves.

Would you mind offering a branch where ESLint can't resolve an import and then another branch where that is solved using eslint-import-resolver-typescript, please?

[mrhyde]

There isn't a need to go to such lengths for something so simple. It essentially boils down to the fact that, without this plugin, ESLint cannot resolve TypeScript modules. Try enabling the import/no-unresolved rule, and you'll see for yourself.

[mightyiam]

Regarding this particular example; doesn't TypeScript make import/no-unresolved unnecessary?

[mrhyde]

It does, but it doesn't cover all scenarios. For example, when you need to run codemodor a simple find and replace that involves import statements. After that, if you run your linter on the staged files, everything will look okay.

[mightyiam]

Sorry, I lost you at typescript "doesn't cover all cases". What cases does it not cover, please? You mentioned codemod and find-replace but those are means of modification — they don't describe a case.

[mrhyde]

You don't want to use tsc as a linting tool because it's significantly slower, especially on large projects. Nor, adding a build step to your pre-commit hooks would negatively impact the development experience.

This PR provides a solution to have the module resolutions checked without actually running the compiler, thereby saving time.

[mightyiam]

May I know how long your compilation takes and how long your linting with this resolver takes, please?

[mrhyde]

It varies from project to project but approximately building is 2 to 4 times slower

[mightyiam]

I feel like I'm looking for absolute numbers.

When an import does not resolve we'd get both the linter and the compiler errors, correct?

[mightyiam]

In an editor, that is.

[mrhyde]

I feel like I'm looking for absolute numbers.

I feel like I need to clarify that it's not about the compiler time per se, but rather about the need to run a compiler just to get those errors. So it's not, for example, a comparison of 10 seconds of compiler time versus 6 seconds of linting time. Instead, it's about 6 seconds versus 16 seconds (compiler + linter) to get the same amount of information that the linter can provide on its own.

Yes, you can see both error messages in your IDE (VSCode in my case), but I have already mentioned scenarios where those messages might be missed.

[mrhyde]

❯ time npm run build tsc --project tsconfig.build.json

user=14.81s system=0.11s cpu=118% total=12.559

❯ time npm run check:code eslint . --ext .ts --report-unused-disable-directives --max-warnings 0

user=5.55s system=0.59s cpu=172% total=3.559

[mightyiam]

So, in your workflow, you found yourself using some tool to modify code. And you want to run ESLint to make sure that that tool did not mutilate import statements. But you're not concerned about other mutilations enough to run tsc, as well? In that case, is this a modification that acts solely on import statements? Does it merely reorganize your file structure?

[mightyiam]

@mrhyde ping

[mrhyde]

That was just an example, but the points I mentioned above still hold.

[mightyiam]

This adds a dependency and some complexity and degrades UX (two errors instead of one in the editor).

I feel it must be justified with at least a marginally common use case.

[mightyiam]

There would also be a performance cost if this generally redundant rule is added.

This config is meant for use together with TypeScript.

Besides this import resolution rule, there are other ESLint rules that we don't enable because TypeScript takes care of those cases already. And ESLint might be faster than tsc for those purposes, as well. What makes this case special? Of course, ESLint can never replace TypeScript.

So we will close this now. If you still disagree, please feel free to try to change our minds.