Suggest installing hardhat-foundry when appropriate#7919
Suggest installing hardhat-foundry when appropriate#7919alcuadrado merged 5 commits intohardhat-foundryfrom
Conversation
🦋 Changeset detectedLatest commit: 421ab91 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
21408d9 to
1f943fd
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds detection of Foundry projects and suggests installing the @nomicfoundation/hardhat-foundry plugin when users encounter uninstalled package errors in projects that use Foundry. The implementation tracks whether a project or npm dependency has a foundry.toml file and uses this information to provide more helpful error messages.
Changes:
- Added
fromHasFoundryTomlfield to error types for tracking Foundry project detection - Enhanced error messages to suggest installing hardhat-foundry plugin when appropriate
- Added comprehensive test coverage for Foundry detection scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| v-next/hardhat/src/types/solidity/errors.ts | Added optional fromHasFoundryToml field to uninstalled package error types |
| v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/error-messages.ts | Updated error messages to suggest hardhat-foundry plugin when Foundry is detected |
| v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/dependency-resolver.ts | Implemented foundry.toml detection logic and propagated the flag through error handling |
| v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/resolver/dependency-resolver.ts | Added test cases for Foundry detection in various scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
3f1dde9 to
32acc40
Compare
alcuadrado
force-pushed
the
hardhat-foundry
branch
from
7fe4e75 to
f68d54c
Compare
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
2610b45 to
36fca98
Compare
alcuadrado
force-pushed
the
hardhat-foundry
branch
from
f68d54c to
ed1b6d6
Compare
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
36fca98 to
a55f120
Compare
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
a55f120 to
c2a1ca9
Compare
| return `${baseMessage} | ||
|
|
||
| Your project has a foundry.toml, and you may need to install the "@nomicfoundation/hardhat-foundry" plugin. | ||
| Learn more about Hardhat's Foundry compatibility here: https://hardhat.org/foundry-compatibility`; |
There was a problem hiding this comment.
this URL is currently a 404! UPDATE: just saw that it is sent as a next PR
| * A boolean indicating if the Hardhat project has a foundry.toml file in its | ||
| * root. | ||
| */ | ||
| fromHasFoundryToml?: boolean; |
There was a problem hiding this comment.
i dont really follow the naming here (not sure what "from" means, maybe we mean the error originated from a project with foundry toml), would "projectHasFoundryToml" or simply "hasFoundryToml" be a clearer name?
There was a problem hiding this comment.
Good point. I renamed this one as projectHasFoundryToml
| * A boolean indicating weather the import is from a source file whose package | ||
| * has a foundry.toml file in its root. | ||
| */ | ||
| fromHasFoundryToml?: boolean; |
There was a problem hiding this comment.
same here about the naming, see my other comment.
There was a problem hiding this comment.
Renamed this as importerPackageHasFoundryToml. The name is more verbose than just packageHas.., but there are to packages in relation to this error, the importer and the imported.
| @@ -57,7 +57,14 @@ Note that the npm module is being remapped by ${formatRemappingReference(error.u | |||
| } | |||
|
|
|||
| case RootResolutionErrorType.NPM_ROOT_FILE_OF_UNINSTALLED_PACKAGE: { | |||
There was a problem hiding this comment.
these errors message updates dont have a corresponding unit test to verify the behavior works with hasFoundryToml and without it. same for the one further below
alcuadrado
force-pushed
the
suggest-hardhat-foundry
branch
from
d80c347 to
421ab91
Compare
This PR detects when there's an import that can't be resolved in a package that has a foundry.toml and suggests reading the docs about it.
Docs PR: NomicFoundation/hardhat-website#215