-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: add watcher to hardhat node #6928
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: b50287a 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 |
hardhatTotal size of the bundle: List of dependencies (sorted by size)
|
We think we can land this after partial recompilation - we need to check that the changed locations for outputs are reviewed. |
c1c5897
to
ccde1db
Compare
ccde1db
to
81cca78
Compare
@kanej this is ready for review. I changed it to use a new internal method vs an rpc method for the edr provider, and adjusted a couple lines related to recent build system changes. |
return; | ||
} | ||
|
||
const buildId = path.basename(absolutePath).replace(".json", ""); |
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.
Is there any implication from the change to include the solc version in the file name?
Do we consider the version to be part of the build id now?
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.
No implications. The build id in this case is used to get the build info filename. But even if it was used for something else, the whole concept of build id now includes the solc version on the id itself, besides the hash
|
||
// NOTE: We listen only to the "add" event because the contents of the build info | ||
// files identified by a build id should be considered immutable under usual circumstances. | ||
watcher.on("add", listener.bind(null, handler)); |
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.
Can I double check, we expect the update to happen when:
- A new contract is added and then a compile done (leading to a completely new build info file)
- An update on an existing contract happens, but this triggers a new build info file?
Or is it the case that we don't expect 2?
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.
When a contract is modified and recompiled, a new build info file is always created. Since build info filenames are a hash of their own content, their content never changes, so no need to listen to update events
… deprecated rpc method
In EDR this no longer returns a bool.
Guard against the server (and EDR instance) being closed before the watcher with its handler built on using the EDR instance is closed.
a05cfa7
to
b50287a
Compare
This replaces #6524
It is a simplified approach to watch for build completion events from
hardhat node
processes. Unlike the original approach, it relies on the fact that the build info output file and the build info file are created sequentially. This is achieved by first creating those files in parallel in the cache directory and moving them one by one to the artifacts directory only after both have been fully written to disk.