Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added WebTorrent add and remove events and docs update #2766

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LostBeard
Copy link

@LostBeard LostBeard commented Mar 27, 2024

What is the purpose of this pull request? (put an "X" next to item)

[X] Documentation update
[ ] Bug fix
[X] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

Added the events 'add' and 'remove' that are fired when a torrent is added to or removed from client.torrents.

This allows attaching to torrent events that may be emitted before the client 'torrent' event is emitted.

I am in a situation where I want to use a custom wire extension on every torrent. I initially tried to listen on the client.on('torrent') event but sometimes the Torrent 'wire' event was firing before the 'torrent' event so it was missed, and my wire extension could not be added via wire.use(). Attaching a 'wire' event handler to the torrent immediately after getting it from client.add() and client.seed() avoided this issue but it meant this had to be done to the torrent after calling client.add or client.seed. Which makes using and sharing custom wire extensions more cumbersome than it needs to be.

Is there anything you'd like reviewers to focus on?

This is a very simple modification. I have added an update to the docs and the code.

Copy link

welcome bot commented Mar 27, 2024

🙌 Thanks for opening this pull request! You're awesome.

@LostBeard
Copy link
Author

I added a WebTorrent bug fix that was made apparent while testing my initial pull request,

Copy link
Member

@SilentBot1 SilentBot1 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me and works as expected in local testing, fixing #2767.

I've suggested some small changes to pass linting, once made, will be happy to approve.

@@ -411,11 +412,14 @@ export default class WebTorrent extends EventEmitter {
_remove (torrent, opts, cb) {
if (!torrent) return
if (typeof opts === 'function') return this._remove(torrent, null, opts)
this.torrents.splice(this.torrents.indexOf(torrent), 1)
var index = this.torrents.indexOf(torrent);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var index = this.torrents.indexOf(torrent);
const index = this.torrents.indexOf(torrent)

torrent.destroy(opts, cb)
if (this.dht) {
this.dht._tables.remove(torrent.infoHash)
}
this.emit('remove', torrent);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.emit('remove', torrent);
this.emit('remove', torrent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants