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

💡 Add eslint-import-resolver-<resolver-name> support #806

Open
ronbraha opened this issue Oct 8, 2024 · 16 comments
Open

💡 Add eslint-import-resolver-<resolver-name> support #806

ronbraha opened this issue Oct 8, 2024 · 16 comments
Labels
feature request Feature request

Comments

@ronbraha
Copy link

ronbraha commented Oct 8, 2024

Suggest an idea for this project

While onboarding Knip on one of the business repositories, I noticed that while the Knip ESLint plugin has broad support for automatically picking up dependencies that ESLint uses (especially with the new flat config), it still does not recognize import resolvers.
Is that something you considered adding support for?

Current day, I have to add eslint-import-resolver-webpack to ignoreDependencies manually.

@ronbraha ronbraha added the feature request Feature request label Oct 8, 2024
@webpro
Copy link
Collaborator

webpro commented Oct 8, 2024

There is some support and coverage for this in the ESLint plugin:

return `eslint-import-resolver-${key}`;

Can you please show a complete reproduction of (relevant) config?

@guillaumebrunerie
Copy link

Same problem here, it worked before with a .eslintrc but then stopped working when I upgraded my ESLint config to a flat config.

Can be reproduced with the following eslint.config.js in a new project:

npm create vite@latest knip-import-resolver-test -- --template vanilla-ts`
cd knip-import-resolver-test
npm install eslint @eslint/js typescript-eslint eslint-plugin-import eslint-import-resolver-typescript knip
// eslint.config.js
import importPlugin from 'eslint-plugin-import';
import js from '@eslint/js';
import tseslint from "typescript-eslint";

export default [
	js.configs.recommended,
	...tseslint.configs.recommended,
	importPlugin.flatConfigs.recommended,
	{
		files: ['**/*.{ts,tsx}'],
		settings: {
			"import/parsers": {
				"@typescript-eslint/parser": [".ts", ".tsx"],
			},
			"import/resolver": {
				typescript: true,
				node: true,
			},
		},
		rules: {
			/* [...] */
		},
	},
];
> npx knip
Unused dependencies (1)
eslint-import-resolver-typescript  package.json

With the corresponding .eslintrc instead, the eslint-import-resolver-typescript dependency is correctly detected:

// .eslintrc
{
	"parser": "@typescript-eslint/parser",

	"settings": {
		"import/parsers": {
			"@typescript-eslint/parser": [".ts", ".tsx"]
		},
		"import/resolver": {
			"typescript": true,
			"node": true,
		},
	},

	"plugins": [
		"import",
		"@typescript-eslint",
	],

	"extends": [
		"eslint:recommended",
		"plugin:import/recommended",
		"plugin:import/typescript",
		"plugin:@typescript-eslint/recommended",
	],
}

@justb3a
Copy link
Contributor

justb3a commented Oct 25, 2024

same here: ("knip": "5.34.0")

> knip

Unresolved imports (3)
@typescript-eslint/parser      .eslintrc.cjs
eslint-plugin-testing-library  .eslintrc.cjs
jsonc-eslint-parser            .eslintrc.cjs

@webpro
Copy link
Collaborator

webpro commented Oct 25, 2024

Thanks for using Knip everyone, but please create an actual issue reproduction. Or rather, open a pull request.

It was my assumption that eslint.config.{js,cjs,mjs} didn't require any custom parsing (like .eslintrc.cjs does) and could just be an entry file, but apparently that's not the case, this config file also requires some parsing to find the used dependencies.

@webpro
Copy link
Collaborator

webpro commented Oct 25, 2024

same here: ("knip": "5.34.0")

> knip

Unresolved imports (3)
@typescript-eslint/parser      .eslintrc.cjs
eslint-plugin-testing-library  .eslintrc.cjs
jsonc-eslint-parser            .eslintrc.cjs

This is a different issue, because only eslint.config.* uses the new flat config.

@guillaumebrunerie
Copy link

Thanks for using Knip everyone, but please create an actual issue reproduction. Or rather, open a pull request.

There is a actual reproduction in my comment just above.

@webpro
Copy link
Collaborator

webpro commented Oct 25, 2024

@guillaumebrunerie
Copy link

My comment contains all of the exact commands and files needed to reproduce the issue from scratch. What more do you need?

Sure, I can run the commands again and put it on GitHub, here you go: https://github.com/guillaumebrunerie/knip-import-resolver-test

git clone https://github.com/guillaumebrunerie/knip-import-resolver-test
cd knip-import-resolver-test
npm install
npx knip

@webpro
Copy link
Collaborator

webpro commented Oct 25, 2024

Not trying to be pedantic or annoying, but please understand this isn't the first issue reported and it's super helpful in understanding and getting to the root of the issue. Trust me, I did my part by developing and maintaining this tool.

@webpro
Copy link
Collaborator

webpro commented Oct 25, 2024

What more do you need?

Also, this isn't about what I need.

@guillaumebrunerie
Copy link

Not trying to be pedantic or annoying, but please understand this isn't the first issue reported and it's super helpful in understanding and getting to the root of the issue. Trust me, I did my part by developing and maintaining this tool.

I was trying to be helpful by providing a reproduction starting from npm create vite and an eslint.config.js file, rather than dumping a full repository, to make it clear that it has nothing to do with other files that a repository typically contains (like tsconfig.json files and whatnot), while making it still very easy to reproduce locally from scratch.

@webpro
Copy link
Collaborator

webpro commented Oct 28, 2024

I've extended the ESLint plugin for eslint.config.* files:

  • Added .ts, .mts and .cts extensions to be picked up as ESLint config files
  • Returning settings from flat configs (while leaving out '@typescript-eslint/parser, this is different as per the docs of typescript-eslint).

It's a basis for further development, feel free to let me know what else should be included as well.

Might open a little can of worms here, as the plugin now actually loads the eslint.config.* files (previously only statically parsed it). Would be great if you could try it out before release with something like:

npm i -D https://pkg.pr.new/knip@c6d5c10

@ronbraha
Copy link
Author

I've extended the ESLint plugin for eslint.config.* files:

  • Added .ts, .mts and .cts extensions to be picked up as ESLint config files
  • Returning settings from flat configs (while leaving out '@typescript-eslint/parser, this is different as per the docs of typescript-eslint).

It's a basis for further development, feel free to let me know what else should be included as well.

Might open a little can of worms here, as the plugin now actually loads the eslint.config.* files (previously only statically parsed it). Would be great if you could try it out before release with something like:

npm i -D https://pkg.pr.new/knip@c6d5c10

Thanks! I can confirm that this solved the issue.
I will wait for an official release to upgrade

@webpro
Copy link
Collaborator

webpro commented Oct 28, 2024

Thanks for confirming!

Might open a little can of worms here, as the plugin now actually loads the eslint.config.* files

Shouldn't be an issue actually, just realized ESLint itself also uses jiti for loading this file.

@guillaumebrunerie
Copy link

Works fine here too, thanks!

@webpro
Copy link
Collaborator

webpro commented Oct 30, 2024

Update: there's an issue with loading eslint.config.js files, please see #818 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

No branches or pull requests

4 participants