Improve waffle warning and migration guides #2976
Conversation
HH-937 Improve the warning that we print if you have waffle installed
It's not clear from the message how it should fixed. Ideally, we should link to an ad-hoc section of the site, perhaps https://hardhat.org/hardhat-chai-matchers/docs/migrate-from-waffle |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 8fa00c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Thanks for submitting this PR! Unfortunately, it has some linter errors, so we can't merge it yet. Can you please fix them? Running yarn |
| :::tab{value="npm 7+"} | ||
|
|
||
| ``` | ||
| npm uninstall @nomiclabs/hardhat-waffle ethereum-waffle @nomiclabs/hardhat-ethers @nomiclabs/hardhat-etherscan chai ethers hardhat-gas-reporter solidity-coverage @typechain/hardhat typechain @typechain/ethers-v5 @ethersproject/abi @ethersproject/providers |
There was a problem hiding this comment.
We remove everything to avoid potential conflicting versions when the toolbox is later installed. For example, it might happen that the user has an old version of @typechain/ethers-v5 than the peer dependency of the toolbox, which causes issues.
| :::tab{value=yarn} | ||
|
|
||
| ``` | ||
| yarn remove @nomiclabs/hardhat-waffle ethereum-waffle |
There was a problem hiding this comment.
For yarn we can't do a big remove like we do with npm, because removing a package that is not installed causes an error. Since yarn doesn't install peer dependencies and the next command just adds everything needed, the problem mentioned in the previous comment is not relevant here, so this is fine.
There was a problem hiding this comment.
A few suggestions, but overall LGTM.
@alcuadrado may I suggest you double check the waffle incompatibility check, which was removed from
packages/hardhat-chai-matchers/src/internal/checkIfWaffleIsInstalled.ts
and added to both
packages/hardhat-chai-matchers/src/internal/hardhatWaffleIncompatibilityCheck.ts and
packages/hardhat-waffle/src/hardhatChaiMatchersIncompatibilityCheck.ts,
and invoked from
packages/hardhat-waffle/src/index.ts.
docs/src/content/hardhat-runner/docs/guides/migrating-from-hardhat-waffle.md
Outdated
Show resolved
Hide resolved
docs/src/content/hardhat-runner/docs/guides/migrating-from-hardhat-waffle.md
Outdated
Show resolved
Hide resolved
docs/src/content/hardhat-runner/docs/guides/migrating-from-hardhat-waffle.md
Outdated
Show resolved
Hide resolved
docs/src/content/hardhat-chai-matchers/docs/migrate-from-waffle.md
Outdated
Show resolved
Hide resolved
Co-authored-by: F. Eugene Aumson <feuGeneA@users.noreply.github.com>
|
test |
There are two important changes in this PR.
The first one is how the docs talk about migrating from hardhat-waffle. The previous version was confusing, because some sections were about migrating only to the chai matchers, and others about migrating to the toolbox. Now migrating to the toolbox is the default and more prominent recommendation. The guide about just migrating from waffle to the chai matchers is still there, but it's not the default path.
The second change is how and when the warning about waffle is shown. We set a global variable in both plugins, and each plugin checks if the other was loaded. If that happens, we throw an error.
edit: test