Skip to content
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

Replace network state with networkId and networkStatus #1196

Merged
merged 4 commits into from
Apr 18, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Apr 17, 2023

Description

The network controller network state used to be set to loading if the network was loading or uninitialized, or to the network ID if the network had finished loading. Effectively it was tracking both the network ID for the current selected network, and the network status.

This state property has been split in two; now we track the network ID of the current selected network separately from the network status. The network status has been expanded to include more states as well.

Changes

  • BREAKING: The network state has been replaced by networkId and networkStatus
    • If you were using network to access the network ID, use networkId now instead. It will be set to null rather than loading if the network is not currently available.
    • If you were using network to see if the network was currently available, use networkStatus instead. It will be set to NetworkStatus.Available if the network is available.
    • When the network is unavailable, we now have two different states to represent that: "Unknown" and "Unavailable". Unavailable means that we know the network is not currently available, whereas unknown is used for unknown errors and for cases where we don't yet know the network status (e.g. before initialization, or while the network is loading).

References

Relates to #1020

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@Gudahtt Gudahtt force-pushed the replace-network-with-network-status branch 5 times, most recently from 087e0f5 to 03739b1 Compare April 17, 2023 22:27
The network controller `network` state used to be set to `loading` if
the network was loading or uninitialized, or to the network ID if the
network had finished loading. Effectively it was tracking both the
network ID for the current selected network, and the network status.

This state property has been split in two; now we track the network ID
of the current selected network separately from the network status. The
network status has been expanded to include more states as well.

Closes #1020
@Gudahtt Gudahtt force-pushed the replace-network-with-network-status branch from 03739b1 to fb98499 Compare April 17, 2023 22:28
@Gudahtt Gudahtt marked this pull request as ready for review April 17, 2023 22:35
@Gudahtt Gudahtt requested a review from a team as a code owner April 17, 2023 22:35
Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
@Gudahtt Gudahtt mentioned this pull request Apr 18, 2023
3 tasks
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Makes sense! Just had a single suggestion.

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Great!

@Gudahtt Gudahtt merged commit d73498e into main Apr 18, 2023
@Gudahtt Gudahtt deleted the replace-network-with-network-status branch April 18, 2023 19:23
@legobeat legobeat mentioned this pull request Apr 25, 2023
Gudahtt added a commit that referenced this pull request May 10, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
Gudahtt added a commit that referenced this pull request May 10, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
Gudahtt added a commit that referenced this pull request May 10, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
Gudahtt added a commit that referenced this pull request May 10, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Replace `network` state with `networkId` and `networkStatus`

The network controller `network` state used to be set to `loading` if
the network was loading or uninitialized, or to the network ID if the
network had finished loading. Effectively it was tracking both the
network ID for the current selected network, and the network status.

This state property has been split in two; now we track the network ID
of the current selected network separately from the network status. The
network status has been expanded to include more states as well.

Closes #1020

* Clarify description

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>

* Adjust types used in tests

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Replace `network` state with `networkId` and `networkStatus`

The network controller `network` state used to be set to `loading` if
the network was loading or uninitialized, or to the network ID if the
network had finished loading. Effectively it was tracking both the
network ID for the current selected network, and the network status.

This state property has been split in two; now we track the network ID
of the current selected network separately from the network status. The
network status has been expanded to include more states as well.

Closes #1020

* Clarify description

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>

* Adjust types used in tests

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The network change handler for the ENS controller has been broken since
the PR #1170 due to a conflict with #1196, which was merged around the
same time. It referenced the `network` property of the network state
that we have removed.

The change handler has been updated to use `networkId` instead.
Additionally, the `NetworkState` type has been imported so that we're
less likely to make this mistake again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants