atmina / linting

A collection of opinionated in-house linting rules.
MIT License
3 stars 2 forks source link

RFC: tone down `eslint-plugin-import` #210

Open reiv opened 6 months ago

reiv commented 6 months ago

The problem with import/no-unresolved, which warns against (supposedly) invalid imports, is that it often runs into false positives, because it essentially needs to mimic in its own little world what the import actually does. To do this reliably, it needs special resolvers (e.g. TypeScript aliases, Webpack and Vite asset imports). This is a fractured ecosystem and has even lead to a fork of the plugin itself (eslint-plugin-i). And when it doesn't work, it is very annoying and confusing when ESLint is not in agreement with what the editor/IDE shows.

I think the cost of keeping eslint-plugin-import in sync with whatever the bundler is doing under the hood isn't worth the benefit. In fact, this might not be something that should be part of the linter config at all (kind of like how ESLint doesn't do formatting for us because we have Prettier, imports should be a concern of TypeScript or the bundler). To take the point further, a passed ESLint check does not guarantee that tsc will succeed, so there will always be a need for both.

For this reason I am suggesting that we disable the no-unresolved rule. The other rules which we use as part of the recommended preset should still work as long as the plugin can resolve the import, but it should shut up if it can't and let the big boys do the talking. For reference, these rules are:

import/default: Ensure a default export is present, given a default import.

import/named: Ensure named imports correspond to a named export in the remote file.

import/namespace: Ensure imported namespaces contain dereferenced properties as they are dereferenced.

Regarding the idea of outright removing this plugin: unfortunately we lose import/order with it and to be honest I am pretty fond of that one because it is nice not having to think about import order at all while still having some kind of structure that isn't just "first come first served" (I might get contested on this :) )

Unfortunately the built-in sort-imports auto-fix does not do line reordering (https://eslint.org/docs/latest/rules/sort-imports).

There is an alternative which looks to be more powerful (and opinionated): https://github.com/lydell/eslint-plugin-simple-import-sort but switching to it will most likely be a breaking change.

mvarendorff2 commented 6 months ago

I agree with all points mentioned; imports should be a concern of both the typechecker (since it needs the imports for compiling) and the bundler, not the linter. Keeping import/order is a good idea, since it falls in line with the setups we use in other projects (like organize imports for dart projects).