Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

adding unsubscribe(eventName) #436

Closed
Isan-Rivkin opened this issue Feb 6, 2019 · 3 comments
Closed

adding unsubscribe(eventName) #436

Isan-Rivkin opened this issue Feb 6, 2019 · 3 comments

Comments

@Isan-Rivkin
Copy link
Contributor

Isan-Rivkin commented Feb 6, 2019

Hi, currently unsubscribe method must take a reference to the handler and it forces me to keep a reference to it while it may not be ideal for some implementations.
Maybe unsubscribe(eventName) could be provided based on EventEmitter.removeAllListeners(eventName) and how floodsub is implemented (inherits EventEmitter).
I don't mind creating PR for this, just wonder if there's an issue with that first,

@Isan-Rivkin Isan-Rivkin changed the title adding removeAllIsteners(eventName) adding unsubscribe(eventName) Feb 6, 2019
@alanshaw
Copy link
Contributor

alanshaw commented Feb 6, 2019

I think that's a good suggestion. We'd need a few PRs to make this happen:

  1. PR to add a test in this repo and update the documentation
  2. PR to js-libp2p https://github.com/libp2p/js-libp2p/blob/4ed5c039fc8583b62cb1e41284a025f5d8d258bc/src/pubsub.js#L41 to call removeAllListeners instead if handler == null
  3. PR to ipfs-http-client to do the same thing https://github.com/ipfs/js-ipfs-http-client/blob/bfd71c2bd9a467b166c85eaa7dd5d7fb94e4dfa4/src/pubsub.js#L80

@jacobheun @vasco-santos is that cool with you?

@jacobheun
Copy link
Contributor

@jacobheun @vasco-santos is that cool with you?

Yes, this sounds great.

@Isan-Rivkin
Copy link
Contributor Author

moved to PR #437

@ghost ghost removed the ready label Feb 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants