Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: add ignore options for import/no-duplicates #1479

Open
JounQin opened this issue Sep 17, 2019 · 35 comments
Open

feature request: add ignore options for import/no-duplicates #1479

JounQin opened this issue Sep 17, 2019 · 35 comments

Comments

@JounQin
Copy link
Collaborator

JounQin commented Sep 17, 2019

When using TypeScript with date-fns, it only has one bundled typings.d.ts, so eslint-import-resolver-ts will resolve all modules to it.

But they are not all exported from date-fns, what means date-fns and date-fns/locale are different.

So we should allow this case by whitelist.

@ljharb
Copy link
Member

ljharb commented Sep 17, 2019

This seems like an inherent flaw in resolving a module to a d.ts file that might represent multiple files. I don’t think forcing an allowlist is an elegant solution; I’d prefer to find a better one.

@JounQin
Copy link
Collaborator Author

JounQin commented Sep 17, 2019

If we do have a better one, that will be greater.

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 21, 2021

@ljharb Any news on this. If we don't have a better solution, an option for whitelist maybe considered?

@ljharb
Copy link
Member

ljharb commented Feb 21, 2021

Rereading the OP, it kind of seems like it’s just a bug in date-fns’ typings file?

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 21, 2021

Why do you think it's a bug of date-fns? It's valid usage of .d.ts files.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2021

But they are not all exported from date-fns, what means date-fns and date-fns/locale are different.

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 21, 2021

.d.ts are fine to be declared in one file for multiple sources. It valid and TypeScript itself doesn't complain. So it's just a choice.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2021

If the types in date-fns are correct, then eslint-import-resolver-ts (eslint-import-resolver-typescript is the one i recommend, incidentally) should be resolving them properly.

I'm still confused what the issue is. Are you saying that because date-fns and date-fns/locale resolve to the same type file, they're considered as duplicate imports?

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 21, 2021

import * from 'date-fns'
import * from 'date-fns/locale'

The above import statements both resolves a same typings.d.ts which include

declare module 'date-fns' {
  // ...
}

declare module 'date-fns/locale' {
  // ...
}

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 21, 2021

Are you saying that because date-fns and date-fns/locale resolve to the same type file, they're considered as duplicate imports?

Yes, that's the point.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2021

It doesn’t seem right to me that they should resolve to the location of their types, so that seems like something that should be fixed in the resolver.

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 22, 2021

What's the problem of the resolver? We may import types from the pkg, there is no types in .js sources, so .d.ts files are preferred.

@ljharb
Copy link
Member

ljharb commented Feb 22, 2021

Yeah, i can see the difficulty. I’m not sure how we’d solve this properly. An exclude list seems like a workaround.

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 22, 2021

An exclude list seems like a workaround.

So is this accepted? I can raise a PR if so.

@ljharb
Copy link
Member

ljharb commented Feb 22, 2021

Given that most packages do the right thing, and don't ship types directly but rather use DT, and given that this is a potential problem with any package that chooses to conflate its API's semver with the semver of its types, I'd prefer to not add an exclude list and instead seek a true fix.

@JounQin
Copy link
Collaborator Author

JounQin commented Feb 22, 2021

So we need to support resolve .d.ts with original .js source at the same time, right?

@ljharb
Copy link
Member

ljharb commented Feb 23, 2021

I think that is the challenge, yes.

@wcastand
Copy link

Hi, we have the issue where --fix merge both:

import { format } from 'date-fns';
import fr from 'date-fns/locale/fr';

is merged into

import fr, { format } from 'date-fns';

Is there a way to avoid this since the last msg is from more than a year ago?
Right now we have to disable eslint on it which doesn't really look like a solution :/

@natterstefan
Copy link

Hi @wcastand,

our workaround for this, until its solved, looks like this:

/* eslint-disable import/no-duplicates */
import formatDate from 'date-fns/format'
import enGB from 'date-fns/locale/en-GB'
import de from 'date-fns/locale/de'
/* eslint-enable import/no-duplicates */

I hope this helps.

@wcastand
Copy link

yeah we went with a ignore next line on each line but it's not really a solution to me. thanks for sharing still :)

@manfrinmm
Copy link

This was my temporary solution:

import { differenceInDays, format, startOfDay, startOfToday } from "date-fns";
import * as locale from "date-fns/locale";

...

const { ptBR } = locale;

@lucasdu4rte
Copy link

@manfrinmm import * as locale from "date-fns/locale" imports all locales available in date-fns, increasing the build size.

that's my temporary solution:

// locale/dateFns.ts
import ptBRLocale from 'date-fns/locale/pt-BR';

export { ptBRLocale };

// other files
import { ptBRLocale } from './locale/dateFns';

@trajano
Copy link

trajano commented Jan 22, 2023

at this rate especially because of this bug I think using something that utilizes the typescript language server may be more prudent (e.g. prettier-plugin-organize-imports). I just got dinged by this when I was setting up the ESLint rules for my monorepo

@alexilyaev
Copy link

Another simple use-case:

import format from 'date-fns/format';
import differenceInHours from 'date-fns/differenceInHours';
import orderBy from 'lodash/orderBy';
import get from 'lodash/get';

image

'.../node_modules/date-fns/typings.d.ts' imported multiple times

Would love to have an options to exclude some packages or patterns from the check

@mvshmakov
Copy link

A temporary workaround can be to use eslint-plugin-local and define a custom post-processor:

plugins: ['local'],
processor: 'local/suppress',

Then, to create a .eslintplugin.js file in the root of the project with the following content:

module.exports = {
  processors: {
    suppress: {
      postprocess: (messages) => {
        const [message] = messages.map((violations) =>
          violations.filter(
            (violation) =>
              !violation.message.endsWith(
                "date-fns/typings.d.ts' imported multiple times."
              )
          )
        )
        return message
      }
    }
  }
}

But a proper support of it is still highly desirable. Either with an array of excludes, or a proper support on the resolver level.

@EmCeeEs
Copy link

EmCeeEs commented Nov 1, 2023

Same thing happens with svelte:

  import { onMount } from "svelte"
  import { slide } from "svelte/transition"

will be fixed to yield

  import { onMount, slide } from "svelte"

@ShadiestGoat
Copy link

Same problem here. Would love a resolution!

This can probably be paired with eslint-import-resolver-typescript#197 as a 'refactor the way .d.ts modules are handled' issue

@JounQin
Copy link
Collaborator Author

JounQin commented Nov 20, 2023

This can probably be paired with import-js/eslint-import-resolver-typescript#197 as a 'refactor the way .d.ts modules are handled' issue

You will meet this issue more often if custom .d.ts with declare is supported.

@ShadiestGoat
Copy link

ShadiestGoat commented Nov 20, 2023

This can probably be paired with import-js/eslint-import-resolver-typescript#197 as a 'refactor the way .d.ts modules are handled' issue

You will meet this issue more often if custom .d.ts with declare is supported.

Not if its properly supported

@JounQin
Copy link
Collaborator Author

JounQin commented Jan 17, 2024

I may do some prototypes on https://github.com/un-es/eslint-plugin-i

@Keavon
Copy link

Keavon commented Mar 13, 2024

The original post and title seems to be rather different from the problem we're all facing here. While an ignore list might be useful, the more pressing problem seems to be a request to fix this bug where multiple declare modules in the same .d.ts file are being treated as one, causing the --fix to even break our code. Should this issue be updated a bit to reflect the new reality of the request? (CC: @JounQin @ljharb)

@nonameolsson
Copy link

nonameolsson commented Mar 13, 2024

**Keavon ** commented Mar 13, 2024

I agree, the description in this issue seems to reflect the problem we are having in this thread.

Not sure why, but we are in the process of upgrading from Svelte 3 to Svelte 4, and now we are facing this error.

Example: This is triggering a warning from the plugin, and is refactored when using fix. And it breaks the code.

  import { createEventDispatcher } from 'svelte';
  import { writable } from 'svelte/store';

After fix:

  import { createEventDispatcher, writable } from 'svelte';

@Keavon
Copy link

Keavon commented Mar 13, 2024

Yes, this issue started happening for me once I upgraded to Svelte 4. Maybe they changed their .d.ts to consolidate the different modules into the same file.

So it should be clear: this problem incorrectly acts in a way that breaks code (when auto-fixing is applied) for anyone using Svelte 4, among many other libraries, which choose to use valid TypeScript for .d.ts files. It seems pretty clear-cut that this should be considered a high-priority bug. I'm confused by the older comments early in this thread which are even debating if it should be fixed?

@nonameolsson
Copy link

Yes, this issue started happening for me once I upgraded to Svelte 4. Maybe they changed their .d.ts to consolidate the different modules into the same file.

So it should be clear: this problem incorrectly acts in a way that breaks code (when auto-fixing is applied) for anyone using Svelte 4, among many other libraries, which choose to use valid TypeScript for .d.ts files. It seems pretty clear-cut that this should be considered a high-priority bug. I'm confused by the older comments early in this thread which are even debating if it should be fixed?

I agree. I'm actually in the process of having to temporarily remove the usage of this ESLint plugin, to be able to upgrade to Svelte 4.
I don't have the knowledge on how to fix this, so I can't submit a PR for this. Hopefully someone else could help with that. I'm happy to test and provide any feedback to make this possible.

@Keavon
Copy link

Keavon commented Mar 13, 2024

You can just disable this specific check if you'd prefer:

// Reenable once <https://github.com/import-js/eslint-plugin-import/issues/1479> is fixed.
"import/no-duplicates": "off",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests