#167 wrtc fix and updated examples to work in esm

[konsumer]

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

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

What changes did you make? (Give an overview)

I swapped out wrtc with @koush/wrtc which works on Mac M1s, and builds (currently there is a node-gyp error in wrtc, #167 ) Additionally, the examples use require and __filename even though the lib is set to type: module so they wouldn't run (and even renaming them to .cjs didn't work, so I just updated them to use esm syntax.)

Which issue (if any) does this pull request address?

#167

[welcome]

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

[socket-security]

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue	Package	Version	Note	Source
Install scripts	@koush/wrtc	0.5.3	Install script: install Source: node scripts/download-prebuilt-or-build-from-source.js	package.json

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

• @SocketSecurity ignore @koush/wrtc@0.5.3

[ThaUnknown]

did you test if this can connect with web peers?

[konsumer]

did you test if this can connect with web peers?

yep, it connects with web-peers, that is the problem I was trying to solve, since webtorrent package needs a couple things to make it work (which I think is the purpose of webtorrent-hybrid.)

This library is essentially a very small wrapper around webtorrent default, with some globals polyfilled, which make me wonder how much it would take to polyfill in the upstream library (like auto-detect there is no globalThis.WRTC and do a dynamic import), or just include the wrapper there, like in a separate file, like this:

import WebTorrent from 'webtorrent/node.js'

This PR just puts this lib in the same shape it was, but it might be something to think about. Sometimes when I am mixing runtimes (browser, deno, CF edge workers, node, etc) I will just be very careful about "import side-effects" and put imports before they are used, like here is an example with fetch which may or may not be a global, so here is how to use my lib cross-cf if it's not:

// polyfill for fetch
import fetch from 'cross-fetch'

// this is imported, but doesn;t call fetch on import, so it's fine
import { CrossKV } from 'cross-cf'

// set the global, so when it needs it, it's there
globalThis.fetch = fetch

// fetch hasn't been called up until now
const MYDB = new CrossKV('MYDB', { target: 'remote', kvID: 'XXX' })
await MYDB.put('coolA', JSON.stringify({ cool: true }))

Looks like WRTC is only used in 1 place so that could just go in the "node instructions", and this wrapper would be unecessary.

import WebTorrent from 'webtorrent'
import { announceList } from 'create-torrent'
import wrtc from '@koush/wrtc'

// this was in global, but is it needed?
globalThis.WEBTORRENT_ANNOUNCE = announceList
  .map(arr => arr[0])
  .filter(url => url.indexOf('wss://') === 0 || url.indexOf('ws://') === 0)

globalThis.WRTC = wrtc

// now do regular webtorrent stuff...

[ThaUnknown]

yeah, this seems like a potential improvement, and might be a good idea, but there's a lot of shit to be looked into, so it's likely this will go stale for quite a while

important shit that needs to be done original vs this PR:

NWjs support comparisons
Electron support comparisons
performance with web peers
speeds with web peers

[konsumer]

@ThaUnknown Yeh, I hear that. At first I was using my fork of this, but in my project I just follow the above, since I'd rather not have a dep on some random fork (even if it's mine) and it means I can use latest published webtorrent, instead of locking into whatever this has deps on. This PR solves the original problem, for others, of this library not being installable.

[ThaUnknown]

you can run these tests yourself and posts results here is you actually want this merged

[konsumer]

you can run these tests yourself and posts results here is you actually want this merged

I am not sure I understand. What are "these tests"? I only updated the broken dep, and fixed the examples. I am using webtorrent in node (working with web client, etc) without this wrapper, so if you don't want it, don't merge it.

[ThaUnknown]

you can run these tests yourself and posts results here is you actually want this merged

I am not sure I understand. What are "these tests"? I only updated the broken dep, and fixed the examples. I am using webtorrent in node (working with web client, etc) without this wrapper, so if you don't want it, don't merge it.

NWjs support comparisons
Electron support comparisons
performance with web peers
speeds with web peers

comparisons before and after these changes, to check if these changes don't remove support from places where webtorrent might have been used, so stuff like electron >=16.0, node arm etc

[konsumer]

NWjs support comparisons
Electron support comparisons
performance with web peers
speeds with web peers

comparisons before and after these changes, to check if these changes don't remove support from places where webtorrent might have been used, so stuff like electron >=16.0, node arm etc

I don't see how this would effect NWjs, electron, etc (they use webtorrent direct with WebRTC in browser, not this wrapper, right?)

webtorrent-hybrid does not install at all on my M1, or any modern version of node. Do you have tests for these things for web speed-comparisons with the old webrtc lib? If you have a look at @koush/wrtc, it's a pretty minimal fork that just updated a few things to make it build.

Overall, I'm not really sure why this is something that you expect me to test for a little wrapper thing I am not using. I don't even use electron or nwjs (it's been years.)

I thought it might help others to make webtorrent-hybrid work, since it's still in various instructions for "do webtorrent in node". That's me just trying to help out, but again, if you're not interested in webtorrent-hybrid building on M1s or any modern version of node (I think > 10 might have issues), then that's totally cool with me. I don't really understand, but you're the one with commit-rights, and I can happily use webtorrent without it.

As you can see with the linked issue above, I also am suggesting it be added to webtorrent-cli, since it also cannot seed for web-peers, currently, and it would be handy to be able to use the official CLI to do this.

[github-actions]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[konsumer]

@feross this is similar to my other PR. It's basically just an update to the WebRTC polyfill to make it work again.

[github-actions]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[haywirez]

@konsumer I was facing the same issues (building on M1, trying to use current node v19) for a new project. Can confirm your fork seems to work with web peers but proper testing of @koush/node-webrtc is near impossible on this platform as neither the original node-webrtc (which hasn't been updated since 2021) or the fork repo builds on an M1. Someone with a different setup or VMs could try.

I don't know how we'd do E2E tests within this project space as webtorrent-desktop uses the webtorrent package, not webtorrent-hybrid...

[github-actions]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[socket-security]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@koush/wrtc	0.5.3	filesystem, shell, environment	+46	2.53 MB	koush

🚮 Removed packages: wrtc@0.4.7

[github-actions]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?