hardhat icon indicating copy to clipboard operation
hardhat copied to clipboard

Feature: add chai matchers for viem

Open johnpm-12 opened this issue 1 year ago • 10 comments

  • [ ] Because this PR includes a bug fix, relevant tests have been included.
  • [x] Because this PR includes a new feature, the change was previously discussed on an Issue or with someone from the team.
  • [ ] I didn't do anything of this.

This adds a new package hardhat-chai-matchers-viem which is forked from hardhat-chai-matchers but supports viem instead of ethers.

I originally planned to update the existing ethers-based package to support both viem and ethers, but that proved to be difficult. So I made it a separate package, similar to how there is hardhat-toolbox and hardhat-toolbox-viem.

I understand that issue #4874 was to make the existing package compatible, so if that's still the plan, no skin off my back if ya'll decide not to merge this. I wrote this because I needed it internally. It was very time consuming. I pushed it to npm if anyone else finds it useful in the meantime: https://www.npmjs.com/package/hardhat-chai-matchers-viem

All existing tests have been updated to use viem instead of ethers, and all tests pass. However, viem with an external hardhat node does not return contract error data in some cases, so the tests to use an external hardhat node are skipped. All tests pass with the in-process hardhat node. I was going to parse the viem error to piece together what I can about the contract error, but I figured it would be better if the hardhat node or viem or hardhat-viem were updated to include the contract error data instead.

Linting succeeds.

I don't know what your process is for publishing to npm, so I'm not sure if anything needs to be done to add a new package there.

If you want to compare this package to the ethers one, you can use git diff --no-index packages/hardhat-chai-matchers/src/index.ts packages/hardhat-chai-matchers-viem/src/index.ts in the root folder, for example. VS Code also has a command to compare 2 local files.

johnpm-12 avatar May 20 '24 02:05 johnpm-12

🦋 Changeset detected

Latest commit: bb45ca8a1e95333fc1d95678b9115a523e560984

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomicfoundation/hardhat-chai-matchers-viem Major

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

changeset-bot[bot] avatar May 20 '24 02:05 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2024 2:40am

vercel[bot] avatar May 20 '24 02:05 vercel[bot]

Hey, thanks for submitting this pr! I'll review it asap and we can discuss how to move forward

alcuadrado avatar May 24 '24 00:05 alcuadrado

Sounds good.

After using this for a few days, I think it might be worth updating the addressable stuff to lowercase (or convert to checksum) any address it find in a .equal or .not.equal matcher before comparison. Since the original ethers version doesn't do that, I won't add it here, but worth discussing adding it to that package too.

It's annoying to have to use viem's getAddress() constantly (or .toLowerCase()) because RPC calls typically return lowercase addresses (contract addresses, wallet address, etc), while viem contract calls return checksummed addresses (contract.read.someAddress()).

johnpm-12 avatar May 24 '24 01:05 johnpm-12

hey, i also wrote something very similar here just last week! my rewrite is significantly more opinionated than this so i think this is a much more suitable match for the existing matchers package, but it also includes significantly more advanced types so maybe worth looking at for ideas. i also added a .toEqualAddress matcher after running into the same checksummed address issue you mentioned.

TateB avatar May 24 '24 01:05 TateB

Hah, wish I knew that before I started this!

At first glance, definitely some useful things in your more opinionated version.

Could always bring the good stuff from your version into the ethers one. Keeping the ethers and viem ones in parity could get annoying. I might take another crack at combining them, if I have the time

johnpm-12 avatar May 24 '24 02:05 johnpm-12

👋 for a new project I switched from ethers to viem and I was expecting a better developer experience. So far it's good, but today I discovered that I cannot use the chai matchers I'm used to, specifically the ones to check if a tx reverted or if it emitted events.

Once it's ready, it would be great to see this PR merged, as it would help devs to better test their contracts (plus I don't want to go back to ethers only for this).

vrde avatar May 31 '24 15:05 vrde

👋 for a new project I switched from ethers to viem and I was expecting a better developer experience. So far it's good, but today I discovered that I cannot use the chai matchers I'm used to, specifically the ones to check if a tx reverted or if it emitted events.

Once it's ready, it would be great to see this PR merged, as it would help devs to better test their contracts (plus I don't want to go back to ethers only for this).

You can use this for now: https://www.npmjs.com/package/hardhat-chai-matchers-viem

johnpm-12 avatar May 31 '24 18:05 johnpm-12

@alcuadrado any updates on this?

johnpm-12 avatar Jun 18 '24 19:06 johnpm-12

Hey @johnpm-12, we are currently in depths of our Hardhat v3 work, a major overhaul. How we approach viem test assertions, either taking this PR as the basis or a different approach is something we will have more visibility on once we are working through our new test subsystem. Sorry for the delay in getting you a full response.

kanej avatar Jun 27 '24 13:06 kanej