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

Updates EIP-1193 #1738

Merged
merged 1 commit into from Feb 6, 2019

Conversation

Projects
None yet
3 participants
@ryanio
Copy link
Contributor

ryanio commented Feb 6, 2019

  • Remove constructor constraint
  • Add isEIP1193
  • Define minimum viable implementation of EventEmitter
* Remove constructor constraint
* Add `isEIP1193`
* Define minimum viable implementation of EventEmitter
@@ -303,6 +307,10 @@ class EthereumProvider extends EventEmitter {
window.addEventListener('message', this._handleJsonrpcMessage.bind(this));
}
static get isEIP1193() {

This comment has been minimized.

@alcuadrado

alcuadrado Feb 6, 2019

The rest of the document describes this as non-static (i.e. line 109), which I think is better.

If you are dealing with an injected provider, checking a static field would be something like window.ethereum.constructor.isEIP1193. Checking a non-static field would be just window.ethereum.isEIP1193, which is cleaner.

That said, I have no strong preference. Just wanted to point out the inconsistency.

@eip-automerger eip-automerger merged commit b9d464a into ethereum:master Feb 6, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -242,6 +244,8 @@ The `eth_requestAccounts` method **MUST** resolve with an array of the account(s

### Events

The provider **SHOULD** extend from `EventEmitter` to provide dapps flexibility in listening to events. In place of full `EventEmitter` functionality, the provider **MAY** provide as many methods as it can reasonably provide, but **MUST** provide at least `on`, `emit`, and `removeListener`.

This comment has been minimized.

@alcuadrado

alcuadrado Feb 6, 2019

What about linking to node.js's docs here? It would help clarify how those methods should behave.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment