Add event typing for common and devp2p
#3753
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
packages/devp2p/src/types.ts
Outdated
| export interface RLPxEvent { | ||
| 'peer:added': Peer | ||
| 'peer:error': { peer: Peer; error: any } | ||
| 'peer:removed': { peer: Peer; reason: any; disconnectWe: any } |
There was a problem hiding this comment.
What is disconnectWe? Is there a more descriptive name for this field?
There was a problem hiding this comment.
It's just saying whether we "disconnected" the connection or the remote peer. I'll add some comments on the types.
| it('DPT: new working node', () => { | ||
| const dpts = util.initTwoPeerDPTSetup(41622) | ||
|
|
||
| dpts[0].events.on('peer:new', async (peer: any) => { |
There was a problem hiding this comment.
It's actually great to see these events typed in a more meaningful way 🙂, have struggled with what "peer" type is being returned when using the API before.
| "test/**/*.ts", | ||
| "examples/**/*.ts", | ||
| "scripts/**/*.ts", | ||
| "examples/simple.cts", |
There was a problem hiding this comment.
Why did these change? Were these examples not being included before?
There was a problem hiding this comment.
We just forgot to update the extension when we migrated over to ESM for our base dev environment. This is just miscellaneous cleanup.
packages/devp2p/src/types.ts
Outdated
| export interface RLPxEvent { | ||
| 'peer:added': [peer: Peer] | ||
| 'peer:error': [peer: Peer, error: any] | ||
| 'peer:removed': [peer: Peer, reason: any, disconnectWe: any] // disconnectWe indicates whether the disconnection was initiated by us or not |
There was a problem hiding this comment.
I typed the disconnectWe as boolean | null. It was typed as that in rlpx/peer.ts and typecheck passes
|
Pushed some minor type improvements and reviewed the code, lgtm! |
3 participants
This PR partially addresses #3750
devp2pandcommoneventsdevp2peth-simulatortest so it's comprehensibleles-simulatortests