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

Implement EventEmitter compatible with browsers #6398

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4fb927f
implement `EventEmitter` compatible with browsers
Muhammad-Altabba Aug 31, 2023
0e9ce5d
some fixes at InBrowserEventEmitter
Muhammad-Altabba Sep 4, 2023
28ba5dc
some renaming inside event_emitter.ts
Muhammad-Altabba Sep 4, 2023
87b2b86
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Sep 7, 2023
ae40b19
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Sep 17, 2023
8913307
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Sep 19, 2023
6d0bd23
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Sep 26, 2023
cb742c7
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Oct 4, 2023
3766eec
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Oct 4, 2023
195463d
export EventEmitter as a class
Muhammad-Altabba Oct 4, 2023
1808058
add unit tests for EventEmitter
Muhammad-Altabba Oct 4, 2023
607f4b2
disable lint rule in a file and update yarn.lock
Muhammad-Altabba Oct 4, 2023
4c8bc1b
apply fixes and add tests for EventEmitter
Muhammad-Altabba Oct 4, 2023
e4d7317
configure `EventEmitter` test to run inside `jsdom`
Muhammad-Altabba Oct 4, 2023
9b0ba51
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Oct 4, 2023
c2eec8e
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
Muhammad-Altabba Oct 6, 2023
b51859b
add Cypress configuration to web3-utils
Muhammad-Altabba Oct 6, 2023
7589433
test EventEmitter in the browser with Cypress
Muhammad-Altabba Oct 6, 2023
dbbdff2
ignore lint for cypress files at web3-utils
Muhammad-Altabba Oct 6, 2023
fe0b27e
Merge branch '4.x' into 6371-uncaught-typeerror-class-extends-value-u…
jdevcs Oct 9, 2023
7697a28
update CHANGELOG.md
Muhammad-Altabba Oct 9, 2023
d7e97db
update CHANGELOG.md files
Muhammad-Altabba Oct 9, 2023
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
2 changes: 1 addition & 1 deletion packages/web3-core/src/web3_event_emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { EventEmitter } from 'events';
import { EventEmitter } from 'web3-utils';

export type Web3EventMap = Record<string, unknown>;
export type Web3EventKey<T extends Web3EventMap> = string & keyof T;
Expand Down
3 changes: 1 addition & 2 deletions packages/web3-eth-accounts/src/common/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/
import pkg from 'crc-32';
import { EventEmitter } from 'events';
import { EventEmitter, bytesToHex, hexToBytes, uint8ArrayConcat } from 'web3-utils';
import type { Numbers } from 'web3-types';
import { bytesToHex, hexToBytes, uint8ArrayConcat } from 'web3-utils';
import { TypeOutput } from './types.js';
import { intToUint8Array, toType, parseGethGenesis } from './utils.js';
import goerli from './chains/goerli.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { EventEmitter } from 'events';
import { EventEmitter } from 'web3-utils';

export default class WebSocket extends EventEmitter {
public readyState: number;
Expand Down
104 changes: 104 additions & 0 deletions packages/web3-utils/src/event_emitter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
This file is part of web3.js.

web3.js is free software: you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

web3.js is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Lesser General Public License for more details.

You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { EventEmitter as NodeEventEmitter } from 'events';

type Callback = (params: any) => void | Promise<void>;

export class InBrowserEventEmitter extends EventTarget {
Copy link
Contributor

@mpetrunic mpetrunic Aug 31, 2023

Choose a reason for hiding this comment

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

Where is this implementation comming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mpetrunic,
This implementation comes from Stackoverflow with the help of ChatGPT 😄 .
Because events module is only a node module that does not exist for the browsers. There is a need to use something different for the browsers. And this class simply uses the class EventTarget available at the browsers and adds more methods to it which we need in our code.
The other alternative to this solution is to use a 3rd party library like: https://www.npmjs.com/package/eventemitter, https://www.npmjs.com/package/events and https://www.npmjs.com/package/eventemitter3. However, I did not want to add an additional dependency.
However, if you have a better approach, kindly advise.
Thanks,

Copy link
Contributor

Choose a reason for hiding this comment

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

Event emmitters are tricky beasts, easy to cause memory leaks or annoying bugs. I would rely on some solid implementation like https://www.npmjs.com/package/eventemitter3

Copy link
Contributor

@jdevcs jdevcs Sep 1, 2023

Choose a reason for hiding this comment

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

This lib is ~70kb unpack size, for now its good to use some dependable lib but also create issue as lib optimisation in future we may attempt reducing lib size then.

Copy link
Contributor Author

@Muhammad-Altabba Muhammad-Altabba Sep 4, 2023

Choose a reason for hiding this comment

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

Thanks @mpetrunic and @jdevcs, for your valid concern and good suggestions.
Because these MR changes still rely on node EventEmitter, when it is a node environment, there are no changes for node js users. And for the browser, it depends on the available class EventTarget by just wrapping it to use it similar to node EventEmitter. So, I see this approach as stable. And, actually, our end-to-end tests pass. But I still need to write unit tests.
However, as you suggested, I can use eventemitter3 for now and create a task to write our own Event Emitter later. This is in case you still see the proposed solution in the MR is not good enough even after writing some unit tests. What do you think?

[There are failed tests for Firefox. But they are not related to the changes in this MR as they exist in other MRs as well]

private _listeners: Record<string, Callback[]> = {};
private maxListeners = Number.MAX_SAFE_INTEGER;

Check warning on line 24 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L23-L24

Added lines #L23 - L24 were not covered by tests

public on(eventName: string, fn: Callback) {
super.addEventListener(eventName, fn as EventListener);
this._addToListeners(eventName, fn);
return this;

Check warning on line 29 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L26-L29

Added lines #L26 - L29 were not covered by tests
}

public once(eventName: string, fn: Callback) {
const onceCallback = async (params: unknown) => {
await fn(params);
this.off(eventName, onceCallback);

Check warning on line 35 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L32-L35

Added lines #L32 - L35 were not covered by tests
Muhammad-Altabba marked this conversation as resolved.
Show resolved Hide resolved
};
return this.on(eventName, onceCallback);

Check warning on line 37 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L37

Added line #L37 was not covered by tests
}

public off(eventName: string, fn: Callback) {
super.removeEventListener(eventName, fn as EventListener);
this._removeFromListeners(eventName, fn);
return this;

Check warning on line 43 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L40-L43

Added lines #L40 - L43 were not covered by tests
}

public emit(eventName: string, params: unknown) {
const event = new CustomEvent(eventName, { detail: params });
return super.dispatchEvent(event);

Check warning on line 48 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L46-L48

Added lines #L46 - L48 were not covered by tests
}

public listenerCount(eventName: string): number {
const eventListeners = this._listeners[eventName];
return eventListeners ? eventListeners.length : 0;

Check warning on line 53 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L51-L53

Added lines #L51 - L53 were not covered by tests
}

public listeners(eventName: string): Callback[] {
return this._listeners[eventName] || [];

Check warning on line 57 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L56-L57

Added lines #L56 - L57 were not covered by tests
}

public eventNames(): string[] {
return Object.keys(this._listeners);

Check warning on line 61 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L60-L61

Added lines #L60 - L61 were not covered by tests
}

public removeAllListeners() {
this._listeners = {};
return this;

Check warning on line 66 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L64-L66

Added lines #L64 - L66 were not covered by tests
}

public setMaxListeners(maxListeners: number) {
this.maxListeners = maxListeners;
return this;

Check warning on line 71 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L69-L71

Added lines #L69 - L71 were not covered by tests
}

public getMaxListeners(): number {
return this.maxListeners;

Check warning on line 75 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L74-L75

Added lines #L74 - L75 were not covered by tests
}

private _addToListeners(eventName: string, fn: Callback) {
if (!this._listeners[eventName]) {
this._listeners[eventName] = [];

Check warning on line 80 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L78-L80

Added lines #L78 - L80 were not covered by tests
}
this._listeners[eventName].push(fn);

Check warning on line 82 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L82

Added line #L82 was not covered by tests
}

private _removeFromListeners(eventName: string, fn: Callback) {
const eventListeners = this._listeners[eventName];
if (eventListeners) {
Muhammad-Altabba marked this conversation as resolved.
Show resolved Hide resolved
const index = eventListeners.indexOf(fn);
if (index !== -1) {
eventListeners.splice(index, 1);

Check warning on line 90 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L85-L90

Added lines #L85 - L90 were not covered by tests
}
}
}
}

// eslint-disable-next-line import/no-mutable-exports
export let EventEmitter: typeof NodeEventEmitter;
// Check if the code is running in a Node.js environment
if (typeof window === 'undefined') {
EventEmitter = NodeEventEmitter;
} else {

Check warning on line 101 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L101

Added line #L101 was not covered by tests
// Fallback for the browser environment
EventEmitter = InBrowserEventEmitter as unknown as typeof NodeEventEmitter;

Check warning on line 103 in packages/web3-utils/src/event_emitter.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/event_emitter.ts#L103

Added line #L103 was not covered by tests
}
1 change: 1 addition & 0 deletions packages/web3-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

export * from './converters.js';
export * from './event_emitter.js';

Check warning on line 19 in packages/web3-utils/src/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/web3-utils/src/index.ts#L19

Added line #L19 was not covered by tests
export * from './validation.js';
export * from './formatter.js';
export * from './hash.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/web3-utils/test/unit/socket_provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { EventEmitter } from 'stream';
import { Web3APIPayload, EthExecutionAPI, JsonRpcResponse, Web3ProviderStatus } from 'web3-types';
import { EventEmitter } from '../../src/event_emitter';
// eslint-disable-next-line import/no-relative-packages
import { sleep } from '../../../../fixtures/utils';
import { SocketProvider } from '../../src/socket_provider';
Expand Down
Loading