eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

[import/no-duplicates] `{"prefer-inline": true}` does not work

Open blackblvck opened this issue 2 years ago • 14 comments

version 2.27.5

https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports The documentation states that adding {"prefer-inline": true} as shown below will cause errors and autofixes if there are duplicate imports. However, this was not possible in my environment.

{
  "import/no-duplicates": ["error", {"prefer-inline": true}],
}

It should look like the following, but there is no error or anything.

import NextErrorComponent from 'next/error';
import { type ErrorProps } from 'next/error';
// ↓
import NextErrorComponent, { type ErrorProps } from 'next/error';

image

Is this currently being corrected?

blackblvck avatar Feb 12 '23 04:02 blackblvck

I agree this should be autofixed - as well as warned on. cc @snewcomer

ljharb avatar Feb 12 '23 05:02 ljharb

Ok perfect. Been working on and throwing away solutions for a few weeks now here and there. Will try to put up my progress today.

snewcomer avatar Feb 12 '23 17:02 snewcomer

Related issue here, when running ESLint from the terminal I get the error

Configuration for rule "import/no-duplicates" is invalid:
Value {"prefer-inline":true} should NOT have additional properties.

even though I used the exact same options as in the docs.

guillaumebrunerie avatar Feb 13 '23 08:02 guillaumebrunerie

The issue lies in the creation of the various import maps in the rule. In the case of the following:

import { type Foo } from '.';
import { foo } from '.';

Line 1 will be sorted into namedTypesImported and Line 2 will be imported into imported, and then when the checkImports function is run later neither of each of the maps' nodes will have length > 1. Currently the rule's example only works because both lines have type imports in them.

If I wrap this block in a if (!prefersInline) check, the above correctly collapses into

import { foo , type Foo } from '.';

I need to dig more though as I'm not yet sure which is the correct description of the problem:

  1. It's not correct to ever sort the inline imports into the *TypesImported maps because those are for managing import type ... type-style import statements which can't be directly compared with import ... value-style import statements. In this case, the wrapped block above is not necessary at all?
  2. Imports with inline type imports should get added to multiple maps. Then we compare types against all types and values against all values, but I'm not sure what happens in a multi-way tie (I haven't grokked the full rule enough yet).

It's likely that the full spectrum of cases needs to be first enumerated. Like what happens in the following situation with and without prefers-inline:

import type { Foo } from '.';
import { foo, type bar } from '.';
import { bar } from '.';

Given the existence of import/consistent-type-specifier-style, should this rule ever concern itself with comparing lines 1 and 2?

  • If no, then I think the path forward is 1 above as that rule will first and then this rule can clean up on a second pass.
  • if yes, then I think the path forward is 2 above (or something like it) and augmenting the rule to somehow cross-compare the import spaces.

merrywhether avatar Feb 20 '23 00:02 merrywhether

@merrywhether Could you try and symlink this PR? I still have to run it against a few repos to see if it works but it would be good to get some early signal!

https://github.com/import-js/eslint-plugin-import/pull/2716

snewcomer avatar Feb 20 '23 01:02 snewcomer

@guillaumebrunerie

Related issue here, when running ESLint from the terminal I get the error

Have you updated the eslint-plugin-import to a version that it is supported. We had an issue where the docs were published before the feature was released. In any case, I would avoid upgrading anyways. I made a mistake with the original PR where I still had the typescript-eslint repo linked where I did the bundled work so got a false positive that the amalgamation of PRs worked :(. no-duplicates It is pretty broken right now.

snewcomer avatar Feb 20 '23 17:02 snewcomer

We had an issue where the docs were published before the feature was released.

That's not entirely accurate; docs are always landed on main prior to being released - users need to check the tagged docs (on every repo on github, not just this one)

ljharb avatar Feb 20 '23 17:02 ljharb

Oh, it can very well be that I did not upgrade, I didn't know it was a new feature. But in that case the error message is very confusing, I think it should say something like Unknown rule "import/no-duplicates". or similar.

guillaumebrunerie avatar Feb 20 '23 17:02 guillaumebrunerie

@guillaumebrunerie you'd need to ask eslint about that; we have no control over that error message. In this case, the rule was known, but that schema option was not.

ljharb avatar Feb 20 '23 18:02 ljharb

I think this is similar to https://github.com/import-js/eslint-plugin-import/issues/2675

niklasnatter avatar Mar 01 '23 16:03 niklasnatter

Having this issue too. It only detects an error when an existing import already has types, like this one:

import { AValue, type AType } from './mama-mia'
import type { BType } from './mama-mia'

Gets fixed into this:

import { type AType, AValue, type BType } from "./mama-mia"

However, if first import doesn't have a type import already, like this:

import { AValue } from './mama-mia'
import type { BType } from './mama-mia'

It doesn't work on that case.

kvnwolf avatar May 18 '23 20:05 kvnwolf

Fixed by https://github.com/import-js/eslint-plugin-import/pull/2835?

alecmev avatar Jul 26 '23 18:07 alecmev

related? https://github.com/import-js/eslint-plugin-import/issues/2792

dylang avatar Sep 27 '23 16:09 dylang

help me please..

juunzzi avatar Jul 29 '24 10:07 juunzzi