fix: update event type to extend { [s: string]: any }

[achingbrain]

The Events type has to extend { [s: string]: any }. Hopefully one day TS will ship a typed EventEmitter and this won't be necessary any more.

Fixes a type error:

[10:45:49] tsc [started]
test/utils/events.ts:64:25 - error TS2344: Type 'Events' does not satisfy the constraint '{ [s: string]: any; }'.

64   emitter: EventEmitter<Events>,
                           ~~~~~~

  test/utils/events.ts:63:35
    63 export const awaitEvents = async <Events = GossipsubEvents>(
                                         ~~~~~~~~~~~~~~~~~~~~~~~~
    This type parameter might need an `extends { [s: string]: any; }` constraint.


Found 1 error in test/utils/events.ts:64

[achingbrain]

Build is failing with

Error: R] No matching export in "dist/src/message/rpc.cjs" for import "default"

    dist/src/message/rpc.js:1:7:
      1 │ import cjs from "./rpc.cjs";

This looks related to the removal of protons and switch back to protobufjs - @tuyennhv any idea why this has started failing to build?

[dapplion]

Build is failing with

it also breaks for me

[twoeths]

@achingbrain now it's failing with another error

Error: src/index.ts(511,68): error TS2345: Argument of type 'import("/home/runner/work/js-libp2p-gossipsub/js-libp2p-gossipsub/node_modules/@multiformats/multiaddr/dist/src/index").Multiaddr[]' is not assignable to parameter of type 'import("/home/runner/work/js-libp2p-gossipsub/js-libp2p-gossipsub/node_modules/@libp2p/interface-connection/node_modules/@multiformats/multiaddr/dist/src/index").Multiaddr[]'.

I think we need to change the way to import from protobuf, will revisit this issue after we fix the above

[achingbrain]

@tuyennhv you can merge #339 to fix the multiaddr error.

[twoeths]

@achingbrain I merged the PR for multiaddr type.

Regarding this error, root cause is

This file is considered to be an ECMAScript module because of the "export" keyword here:

    dist/src/message/rpc.cjs:1747:0:
      1747 │ export {};

somehow rpc.cjs added this at the end of the line so the file becomes an es module

export {};

do you know why that line is added during the build process?

[achingbrain]

Not off the top of my head, no. TBH I've got rid of mixed CJS/ESM/TS files and only use protobuf.js via protons everywhere else in the stack because debugging these sorts of tooling clashes is a frequent and painful activity otherwise.

[mpetrunic]

Not off the top of my head, no. TBH I've got rid of mixed CJS/ESM/TS files and only use protobuf.js via protons everywhere else in the stack because debugging these sorts of tooling clashes is a frequent and painful activity otherwise.

Isn't protons now some in speed as protobuf.js?

[twoeths]

Not off the top of my head, no. TBH I've got rid of mixed CJS/ESM/TS files and only use protobuf.js via protons everywhere else in the stack because debugging these sorts of tooling clashes is a frequent and painful activity otherwise.

Isn't protons now some in speed as protobuf.js?

@mpetrunic it's quite comparable to protobuf but not stable enough from the last time I tried, there's a draft PR for it #327

[achingbrain]

The weird thing in all that is that it uses protobuf.js internally for all serialization/deserialization now so it should be comparable.

It basically just outputs a nice easily-compilable ts interface to the data object created from a .proto file.

It does use a more recent version of protobuf.js than this module, so that might be a thing.

[twoeths]

I'd test protons version of gossipsub in lodestar before merging that PR as performance is very important to lodestar, right now it's blocked by ChainSafe/lodestar#4114

cc @wemeetagain

[twoeths]

merging the PR as it happens in https://github.com/ChainSafe/js-libp2p-gossipsub/actions/runs/3103234285/jobs/5026341160

there should be another build issue regarding rpc.cjs file