import-js / eslint-import-resolver-typescript

This plugin adds `TypeScript` support to `eslint-plugin-import`
https://www.npmjs.com/package/eslint-import-resolver-typescript
ISC License
698 stars 61 forks source link

chore: remove `eslint-plugin-import` from peerDeps #248

Closed SukkaW closed 7 months ago

SukkaW commented 8 months ago

The PR removes eslint-plugin-import from the peerDependencies, assuming that the eslint-plugin-i or eslint-plugin-import is most likely already present before installing eslint-import-resolver-typescript.

I am using eslint-import-resolver-typescript with eslint-plugin-i, and I don't want npm/npm to install eslint-plugin-import from peerDependencies automatically.

changeset-bot[bot] commented 8 months ago

⚠️ No Changeset found

Latest commit: 05f95943482464da38190ca26b4a023b95c39603

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

SukkaW commented 8 months ago

@JounQin Should this PR be included in a minor version bump or a patch one?

JounQin commented 8 months ago

eslint-plugin-i is recommended to be used as npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest, this should work without peer issue AFAIK?

Can you link some documents that npm will install peer dependencies automatically?

SukkaW commented 8 months ago

Can you link some documents that npm will install peer dependencies automatically?

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#peerdependencies

Here I quote:

In npm versions 3 through 6, peerDependencies were not automatically installed, and would raise a warning if an invalid version of the peer dependency was found in the tree. As of npm v7, peerDependencies are installed by default.


eslint-plugin-i is recommended to be used as npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest, this should work without peer issue AFAIK?

I have adapted the new ESLint Flat Configuration (which will become the default configuration format of ESLint 9), which allows install and import eslint-plugin-i directly:

https://github.com/SukkaW/eslint-config-sukka/blob/238c5ff109bde19e0e6c92ae40339d43a6338b89/packages/ts/src/index.ts#L10 https://github.com/SukkaW/eslint-config-sukka/blob/238c5ff109bde19e0e6c92ae40339d43a6338b89/packages/ts/src/index.ts#L39-L40

Also, apparently someone else is facing the same issue, see another PR: https://github.com/import-js/eslint-import-resolver-typescript/pull/243

JounQin commented 8 months ago

Here I quote:

If you use npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest, will it still install official eslint-plugin-import anyway?

I have adapted the new ESLint Flat Configuration (which will become the default configuration format of ESLint 9), which allows install and import eslint-plugin-i directly:

I believe npm install eslint-plugin-import@npm:eslint-plugin-i@latest (as dependency) will still work if you change to use import eslint_plugin_import from 'eslint-plugin-import'; instead?

I didn't notice #243 previously, @wojtekmaj does npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest solution work to you?

wojtekmaj commented 8 months ago

@JounQin Oh wow, didn't think about that! Yes, this works!

SukkaW commented 8 months ago

If you use npm install -D eslint-plugin-import@npm:eslint-plugin-i@latest, will it still install official eslint-plugin-import anyway?

Sadly yes. However, that's a different problem: eslint-plugin-next also depends on eslint-plugin-import.

wojtekmaj commented 8 months ago

The answer to that problem, or rather a workaround, is Yarn's resolutions. I believe other package managers have similar features too.

JounQin commented 8 months ago

Sadly yes. However, that's a different problem: eslint-plugin-next also depends on eslint-plugin-import.

So this PR can't resolve such problems, right?

SukkaW commented 8 months ago

Sadly yes. However, that's a different problem: eslint-plugin-next also depends on eslint-plugin-import.

So this PR can't resolve such problems, right?

Well I still want to install eslint-plugin-i directly without alias.

JounQin commented 8 months ago

Well I still want to install eslint-plugin-i directly without alias.

I personally still recommend to use alias instead, because it is an alias. And as you described when using with other config/plugins together, eslint-plugin-import and eslint-plugin-i will be installed together which is a waste. Besides, #243 would be more suitable even if we want to use eslint-plugin-i directly although I don't recommend.

JounQin commented 7 months ago

Close due to inactive

SukkaW commented 7 months ago

I personally still recommend to use alias instead, because it is an alias.

Now installing eslint-plugin-i as an alias introduces way too many problems, the most noticeably one is:

Oops! Something went wrong! :(

ESLint: 8.54.0

TypeError: Cannot redefine plugin "import".
JounQin commented 7 months ago

It seems you're importing i and import at the same time. Maybe you can provide a reproduction.

SukkaW commented 7 months ago

It seems you're importing i and import at the same time. Maybe you can provide a reproduction.

If there is a simple minimum reproduction, I would have provided it already, but there would always be duplicated eslint-plugin-i and eslint-plugin-import in my node_modules thanks to the peerDependencies and transitive dependencies.

JounQin commented 7 months ago

Can you use overrides then?