Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Fix 3380: Add AbstractProvider #3451

Merged
merged 7 commits into from
May 5, 2020
+20 −1
Merged

Fix 3380: Add AbstractProvider #3451

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
79ab9f3
add AbstractProvider
odanado Apr 1, 2020
20491df
add test for AbstractProvider
odanado Apr 1, 2020
7682f03
Merge branch '1.x' into fix-3380
ryanio Apr 17, 2020
151dc85
Merge branch '1.x' into fix-3380
ryanio Apr 27, 2020
d951cb4
Merge branch '1.x' into fix-3380
ryanio May 1, 2020
d3ef78e
add interface of sendAsync
odanado May 4, 2020
28dc745
fix web3-test.ts
odanado May 5, 2020
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
10 changes: 9 additions & 1 deletion packages/web3-core/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@
*/

import * as net from 'net';
import { EventEmitter } from "events"
import {
HttpProviderBase,
HttpProviderOptions,
IpcProviderBase,
WebsocketProviderBase,
WebsocketProviderOptions
WebsocketProviderOptions,
JsonRpcPayload,
JsonRpcResponse
} from 'web3-core-helpers';
import { Method } from 'web3-core-method';
import BN = require('bn.js');
Expand Down Expand Up @@ -408,9 +411,14 @@ export interface LogsOptions {

export type BlockNumber = string | number | BN | BigNumber | 'latest' | 'pending' | 'earliest' | 'genesis';

export interface AbstractProvider extends EventEmitter {
Copy link
Contributor Author

@odanado odanado Apr 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I would like to consider the following points

  • Using the extends EventEmitter.
  • Remove extends EventEmitter (Leaving only the send function in the interface).
  • Remove extends EventEmitter and directly define the on, once, and removeListener functions as well as the WebsocketProviderBase.

Copy link

@smithki smithki Apr 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

My $0.02 is to separate the core functionality from the subscription functionality. Not all custom providers support subscription features by default. Classes can implement multiple interfaces in TypeScript, so it shouldn't be problematic to export two interfaces.

I am not an active contributor to this code-base. This is my opinion only.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @odanado thanks for this PR :) It LGTM. For your 3 points, perhaps just on/once/removeListener is sufficient as the "bare minimum".

What are the types of WebsocketProviderBase?

Also, in light of recent updates to EIP-1193, there is a new request method that would be great to add. I don't think having sendAsync would hurt either even though it's deprecated it has more long term support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryanio Thank you for the comment.
I didn't know the request method of EIP-1193.

I thought that the interface of the AbstractProvider should be an only of send and sendAsync.
The reasons are as follows.

  • The status of EIP-1193 is a draft.
  • EIP-1193 is not yet fully implemented in Metamask

I think it should define a provider that has methods such as on/once/removeListener/request separately from AbstractProvider.

send(payload: JsonRpcPayload, callback: (error: Error | null, result?: JsonRpcResponse) => void): void;
}

export type provider =
| HttpProvider
| IpcProvider
| WebsocketProvider
| AbstractProvider
| string
| null;
9 changes: 9 additions & 0 deletions packages/web3/types/tests/web3-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

import Web3 from 'web3';
import * as net from 'net';
import { AbstractProvider } from 'web3-core';
import { JsonRpcPayload, JsonRpcResponse } from "web3-core-helpers"
import { EventEmitter } from 'events';

// $ExpectType Utils
Web3.utils;
Expand Down Expand Up @@ -85,3 +88,9 @@ web3 = new Web3('https://localhost:5000/', netSocket);

// $ExpectType Web3
web3 = new Web3();

class CustomProvider extends EventEmitter implements AbstractProvider {
send(payload: JsonRpcPayload, callback: (error: Error | null, result?: JsonRpcResponse) => void) {}
}
// $ExpectType Web3
web3 = new Web3(new CustomProvider());