-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Template improvements #5892
Template improvements #5892
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
describe("Lock", function () { | ||
let networkConnection: NetworkConnection<"l1">; | ||
describe("Lock", async function () { | ||
const { viem, networkHelpers } = await network.connect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it cleans up the code. But I do find it surprising - I expect resource initialization code to be in the before/beforeEach.
I was initially concerned that this code might get run even if the describe is skipped, but I can confirm that isn't the case.
Would we use a similar pattern in the mocha
template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mocha
intentionally doesn't support that. So we can't do it there.
Maybe we can initialize the network connection on the first provider call? I think the code style improvement is massive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In reality, given that this is ESM, these calls can be in the top-level of the file.
|
||
describe("Lock", function () { | ||
let networkConnection: NetworkConnection<"l1">; | ||
let networkHelpers: any; // TODO: We need to export this type in @ignored/hardhat-vnext-network-helpers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to export the NetworkHelpers
as an interface in the plugin. It's now a class, but it should be a quick conversation into an interface. Slightly more involved than just copy and replace, because it has some nested objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left a comment on async usage to avoid an explicit NetworkConnection
, but however that gets resolved the other changes make sense.
This PR includes a first round of improvements to the template projects, it's still WIP, but can be merged as is.