Skip to content

Conversation

@ItsNickBarry
Copy link
Contributor

Hardhat is a CLI tool that relies on the presence of a JS config file. The hardhat package may or may not be imported.

This plugin is meant to do two things:

  1. Add hardhat.config.x as an entry.
  2. Skip warnings if hardhat is not imported anywhere.

Is the implementation correct? I'm not sure about the resolve function.

Copy link
Member

@webpro webpro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the plugin! LGTM, but I do have a remark or two.

{
"name": "@fixtures/hardhat",
"dependencies": {
"hardhat": "*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's typically used as dev dep? Seeing --save-dev in their docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. Does that make a difference for production mode?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. In general just better to have the fixtures as close to reality as possible.


const resolve: Resolve = async () => {
return [toDependency('hardhat')];
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardhat package may or may not be imported.

I understand the reasoning here, but if it's indeed not imported anywhere, then perhaps it's used in some other way? E.g. in package.json#scripts or somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some people add scripts, but there's also a hardhat-shorthand package which is installed globally and adds an hh shortcut to replace npx hardhat, yarn run hardhat, etc. It parses the Hardhat config and supports tab completion for builtin and user-defined commands, making it categorically different from scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the global hh bin still requires that hardhat be installed locally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, bit of a bummer but let's roll with it :)

@pkg-pr-new
Copy link

pkg-pr-new bot commented May 17, 2025

Open in StackBlitz

npm i https://pkg.pr.new/knip@1087

commit: 8c19e3f

@@ -0,0 +1,25 @@
import { toDependency } from 'src/util/input.js';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use relative imports

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@webpro webpro merged commit eba5ebf into webpro-nl:main May 19, 2025
29 checks passed
@webpro
Copy link
Member

webpro commented May 19, 2025

Thanks, Nick!

@webpro
Copy link
Member

webpro commented May 20, 2025

🚀 This pull request is included in v5.57.0. See Release 5.57.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

@ItsNickBarry ItsNickBarry mentioned this pull request Jun 3, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants