Migrate from ethjs to ethers
#29
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I forgot to update the readme! Edit: README has been updated; this should be good to go now. |
`ethers` is better maintained than `ethjs`, and we've been migrating many of our packages to use it. This should work equivalently. This will be a breaking change because this will change public properties of the method registry class.
Gudahtt
force-pushed
the
migrate-from-ethjs-to-ethers
branch
from
96ee678 to
9af43c5
Compare
Gudahtt
added a commit
to MetaMask/core
that referenced
this pull request
Feb 11, 2021
The version of `ethjs-query` we're using has an unlisted dependency upon `babel-runtime`, and will crash if it's not present. Currently it's working because `babel-runtime` is being brought in by another dependency (`fetch-mock`), but that dependency is removed in another PR (#340). Until we can migrate away from using this version of `ethjs-query`, `babel-runtime` has been added as an explicit dependency to ensure it is present. `ethjs-query` is a transitive dependency of `eth-method-registry` via `ethjs`. There is a PR open on `eth-method-registry` to replace `ethjs` altogether [1]. Once those are merged and released, we can update `eth-method-registry` here and remove this `babel-runtime` dependency. [1]: MetaMask/eth-method-registry#29
Gudahtt
added a commit
to MetaMask/core
that referenced
this pull request
Feb 11, 2021
The version of `ethjs-query` we're using has an unlisted dependency upon `babel-runtime`, and will crash if it's not present. Currently it's working because `babel-runtime` is being brought in by another dependency (`fetch-mock`), but that dependency is removed in another PR (#340). Until we can migrate away from using this version of `ethjs-query`, `babel-runtime` has been added as an explicit dependency to ensure it is present. `ethjs-query` is a transitive dependency of `eth-method-registry` via `ethjs`. There is a PR open on `eth-method-registry` to replace `ethjs` altogether [1]. Once those are merged and released, we can update `eth-method-registry` here and remove this `babel-runtime` dependency. [1]: MetaMask/eth-method-registry#29
Gudahtt
commented
Feb 12, 2021
|
|
||
| interface DeployedRegistryContract { | ||
| entries (bytes: string): string[]; | ||
| provider: Provider; |
There was a problem hiding this comment.
It might be better to expect an EIP-1193 provider here rather than an ethers provider.
There was a problem hiding this comment.
Moving this back into draft until we come up with a better interface to use here.
|
@Gudahtt could this be revisted? I think removing ethjs across all the dependencies we maintain would be a solid move considering its staleness. |
|
Happy to take this on, but i'd like more clarification on the issue you highlighted. |
|
I think what we want here is a type that represents an EIP-1193 provider. Then we can pass a provider that implements that interface (e.g. an Ethers provider) without being tied to a specific version of that specific library |
MajorLift
pushed a commit
to MetaMask/core
that referenced
this pull request
Oct 11, 2023
The version of `ethjs-query` we're using has an unlisted dependency upon `babel-runtime`, and will crash if it's not present. Currently it's working because `babel-runtime` is being brought in by another dependency (`fetch-mock`), but that dependency is removed in another PR (#340). Until we can migrate away from using this version of `ethjs-query`, `babel-runtime` has been added as an explicit dependency to ensure it is present. `ethjs-query` is a transitive dependency of `eth-method-registry` via `ethjs`. There is a PR open on `eth-method-registry` to replace `ethjs` altogether [1]. Once those are merged and released, we can update `eth-method-registry` here and remove this `babel-runtime` dependency. [1]: MetaMask/eth-method-registry#29
MajorLift
pushed a commit
to MetaMask/core
that referenced
this pull request
Oct 11, 2023
The version of `ethjs-query` we're using has an unlisted dependency upon `babel-runtime`, and will crash if it's not present. Currently it's working because `babel-runtime` is being brought in by another dependency (`fetch-mock`), but that dependency is removed in another PR (#340). Until we can migrate away from using this version of `ethjs-query`, `babel-runtime` has been added as an explicit dependency to ensure it is present. `ethjs-query` is a transitive dependency of `eth-method-registry` via `ethjs`. There is a PR open on `eth-method-registry` to replace `ethjs` altogether [1]. Once those are merged and released, we can update `eth-method-registry` here and remove this `babel-runtime` dependency. [1]: MetaMask/eth-method-registry#29
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ethersis better maintained thanethjs, and we've been migrating many of our packages to use it. This should work equivalently.This will be a breaking change because this will change public properties of the method registry class.