-
Notifications
You must be signed in to change notification settings - Fork 28
Conversation
@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 { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
js-libp2p-interfaces/packages/interfaces/src/events.ts
Lines 78 to 79 in 2d7e06b
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 | |
}) |
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 |
If you remove the |
There was a problem hiding this 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?
Not unless we override Event.type which is a string and there is no generic to override it. dispatchEvent<E extends {type: keyof EventMap} & Event>(event: E): boolean { but it errors since But even if they add typed EventEmitter, we can just remove all other method implementation and leave safeEventDispatch. |
## [@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))
🎉 This PR is included in version @libp2p/interfaces-v3.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version @libp2p/interface-mocks-v9.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
resolves #317