Allow overriding the type of the default network#7805
Conversation
🦋 Changeset detectedLatest commit: da3b1ae 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 |
|
We should double check running tests against a |
michalbrabec
left a comment
There was a problem hiding this comment.
Overriding the default network creates the need to setup a new network for npx hardhat node and then used it with the --network parameter.
#7795 (comment)
30241c0 to
ec03a01
Compare
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #7795 by allowing users to override the type of the default and localhost network configurations. Previously, these networks had their types forced to specific values (default was always edr-simulated and localhost was always http), preventing users from configuring the default network as an HTTP network pointing to an external node.
Changes:
- Modified network configuration extension logic to respect user-specified network types
- Updated tests to verify that network types can now be overridden
- Added comprehensive test cases for overriding both
defaultandlocalhostnetwork types
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| v-next/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/config.ts | Refactored extendUserConfig to conditionally apply defaults based on network type, allowing users to override the type of default and localhost networks |
| v-next/hardhat/test/internal/builtin-plugins/network-manager/hook-handlers/config.ts | Updated test descriptions and added new test cases to verify that network type overrides work correctly for both localhost and default networks |
| .changeset/four-lies-flash.md | Added changeset documenting the patch-level change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I went for an option where we only set the defaults if the network is I also implemented the same logic for |
|
This has been released as part of |
This fixes #7795, but I'm not fully convinced about the change.
The problem is that we are always setting the type of default network to be a simulated network, and we also set some other default values. This makes sense, but it does seem to assume that the network is not of type
http. My concern is that a user's settings could interact in the wrong way with those default values.On the other hand, without this change there's no way to have an http network as the default network, something that was possible in HH2.
One alternative here is to re-introduce the
defaultNetworkconfig, and default it to "default" (yes).Another alternative is to only use those default values if the user doesn't change the type, but that starts feeling a bit too complex.