Skip to content

Add missing hardhat network variables #643

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.

Sign up for GitHub

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

Closed
fredlacs wants to merge 1 commit into from
Closed

Add missing hardhat network variables #643

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions plugins/resources/nomiclabs.utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ function configureNetworkEnv(env, networkName, networkConfig, provider, isHardha
env.config.defaultNetwork = networkName;

env.network = {
Copy link

@wighawag wighawag Jul 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that it still create a new object will cause issue with plugin that modify the network object before this is called.
It is better to simply modify the network object

Copy link
Author

@fredlacs fredlacs Jul 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the fork with this fork and seems to be working well with hardhat deploy :)

If they modify it before, it shouldn't be an issue since we first set all previous object values then only override the needed ones ...env.network.
If you meant using Object.assign I believe the main difference would only be that setters are called on the object. According to a random SO answer: "spread operator will not copy the the source object’s prototype to the target object"

How would you suggest this be implemented? Glad to iterate on it or take a PR heh

Copy link

@wighawag wighawag Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugins could have copied the reference to hre.network before and will later modify an unrelated object. Modifying the network object would be safer. I proposed this in that comment: wighawag/hardhat-deploy#132 (comment)

Copy link
Contributor

@outdoteth outdoteth Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.assign retains the reference to the original object too. I think that is more conventional. But simplicity is always good so maybe @wighawag solution of just doing inplace modifications (env.network.name = networkName etc.) is better.

...env.network,
name: networkName,
config: networkConfig,
provider: provider,
Expand Down