Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

feat: safe dispatch event #319

Merged
merged 2 commits into from
Jan 17, 2023
Merged

feat: safe dispatch event #319

merged 2 commits into from
Jan 17, 2023

Conversation

mpetrunic
Copy link
Member

resolves #317

@mpetrunic
Copy link
Member Author

@achingbrain any idea whats going on with lerna/nx?

@@ -74,6 +74,10 @@ export class EventEmitter<EventMap extends { [s: string]: any }> extends EventTa

return result
}

safeDispatchEvent<Detail>(type: keyof EventMap, detail: CustomEventInit<Detail>): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

is detail the best name for this param?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea, its already ref like that in multiple places

Copy link
Member

Choose a reason for hiding this comment

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

detail is used in other places as it's the option key from the CustomEvent constructor

Copy link
Contributor

Choose a reason for hiding this comment

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

right it seems confusing since this is actually the CustomEvent options object which contains detail

safeDispatchEvent<Detail>(type: keyof EventMap, detail: CustomEventInit<Detail>): boolean {
return this.dispatchEvent(new CustomEvent<Detail>(type as string, detail))

this.components.connectionManager?.safeDispatchEvent<Connection>('peer:connect', {
detail: aToB
})

@achingbrain
Copy link
Member

any idea whats going on with lerna/nx?

Something has shifted in the dependency graph that means nx can't calculate the dependency graph as it trips over this dependency.

That dep version prevents @semantic-release/github hitting request limits when publishing from monorepos with lots of packages like this one - the PR has been open since April!

@achingbrain
Copy link
Member

If you remove the package-lock.json file, nx starts to work again 🙄

Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

The idea of the typed EventEmitter here is just to be a stopgap until TypeScript adds support for typed emitters. It's supposed to mirror the EventTarget interface.

This PR adds a non-standard method, safeDispatchEvent which stops this being a type-safe drop-in replacement for EventTarget - is there no other way?

@mpetrunic
Copy link
Member Author

mpetrunic commented Dec 14, 2022

The idea of the typed EventEmitter here is just to be a stopgap until TypeScript adds support for typed emitters. It's supposed to mirror the EventTarget interface.

This PR adds a non-standard method, safeDispatchEvent which stops this being a type-safe drop-in replacement for EventTarget - is there no other way?

The idea of the typed EventEmitter here is just to be a stopgap until TypeScript adds support for typed emitters. It's supposed to mirror the EventTarget interface.

This PR adds a non-standard method, safeDispatchEvent which stops this being a type-safe drop-in replacement for EventTarget - is there no other way?

Not unless we override Event.type which is a string and there is no generic to override it.
I tried:

  dispatchEvent<E extends {type: keyof EventMap} & Event>(event: E): boolean {

but it errors since type of CustomEvent we sent there is a string and not key in the map.
We could fix that by using overridden CustomEvent but IMHO this is even more disruptive. Example: https://github.com/mizdra/strictly-typed-event-target/blob/master/src/index.ts#L8

But even if they add typed EventEmitter, we can just remove all other method implementation and leave safeEventDispatch.

@achingbrain achingbrain merged commit 8caeee8 into master Jan 17, 2023
@achingbrain achingbrain deleted the feat/safe-event-dispatch branch January 17, 2023 16:46
github-actions bot pushed a commit that referenced this pull request Jan 17, 2023
## [@libp2p/interfaces-v3.3.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interfaces-v3.2.0...@libp2p/interfaces-v3.3.0) (2023-01-17)

### Features

* safe dispatch event ([#319](#319)) ([8caeee8](8caeee8)), closes [#317](#317)

### Trivial Changes

* remove lerna ([#330](#330)) ([6678592](6678592))
@github-actions
Copy link

🎉 This PR is included in version @libp2p/interfaces-v3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Jan 17, 2023
@github-actions
Copy link

🎉 This PR is included in version @libp2p/interface-mocks-v9.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

events dispatchEvent typing
3 participants