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

WebTorrent: dependency hygiene #5490

Open
feross opened this issue Aug 1, 2019 · 8 comments
Open

WebTorrent: dependency hygiene #5490

feross opened this issue Aug 1, 2019 · 8 comments
Assignees
Labels
feature/webtorrent Label for webtorrent related issues QA/No release-notes/exclude

Comments

@feross
Copy link

feross commented Aug 1, 2019

I want to go through the whole webtorrent dependency tree to:

  • De-duplicate dependencies
  • Ensure that we're using the latest version
  • Pruning dependencies that aren't needed in modern JS environments (to reduce risk and increase install speed)

This issue is a placeholder to track this task.

@feross feross added the feature/webtorrent Label for webtorrent related issues label Aug 1, 2019
@feross feross self-assigned this Aug 1, 2019
@feross
Copy link
Author

feross commented Aug 1, 2019

Here's a todo list:

Direct deps to update

  • bitfield@2.0.0
  • electron@5.0.8
  • stream-to-blob@1.0.2

Indirect deps to update

  • torrent-discovery@9.1.1 / debug@3.2.6
  • simple-peer@9.4.0 / readable-stream@2.3.6
  • create-torrent@4.0.0 / filestream@4.1.3 / readable-stream@2.3.6
  • chunk-store-stream@4.0.0 / block-stream2@1.1.0 / readable-stream@2.3.6
  • bittorrent-tracker@9.11.0 / simple-websocket@7.2.0 / debug@3.2.6
  • bittorrent-tracker@9.11.0 / simple-websocket@7.2.0 / readable-stream@2.3.6

Outdated packages to attempt to remove from tree

These packages are not necessary in modern JS environments anymore.

  • xtend (use Object.assign instead)
  • inherits (use class inheritance instead)
  • safe-buffer (this is my package but it makes sense to use Buffer directly instead now that we got the Buffer vulnerabilities fixed directly in Node.js)
  • buffer-alloc (use Buffer directly)
  • buffer-from (use Buffer directly)
  • buffer-equals (use Buffer.equals directly)
  • mkdirp (use built-in Node.js function)
  • flatten (prints a deprecation warning, replace it)
  • rusha (just use browser crypto api async-only)

Other changes

  • Update stream-http version used in browserify (removes another duplicated version of readable-stream)

Some of these don't actually run in Brave's WebTorrent extension (since they only run when WebTorrent runs in a Node.js environment). Still, we're installing extra dependencies that we don't need, so it's still worthwhile to eliminate these.

feross added a commit to DamonOehlman/filestream that referenced this issue Aug 2, 2019
feross added a commit to webtorrent/create-torrent that referenced this issue Aug 2, 2019
@feross
Copy link
Author

feross commented Aug 2, 2019

Not finished yet, and already some good results:

Before:

$ npm ls --depth=99 --production | wc -l
     267

$ npm run size
   98338

After:

$ npm ls --depth=99 --production | wc -l
     227

$ npm run size
   95467

@kjozwiak
Copy link
Member

kjozwiak commented Aug 2, 2019

@feross labelling this as QA/No. However, if there's anything QA needs to check once this has been completed, please feel free to add test cases and change the label to QA/Yes.

feross added a commit to webtorrent/webtorrent that referenced this issue Aug 4, 2019
feross added a commit to webtorrent/webtorrent that referenced this issue Aug 4, 2019
feross added a commit to feross/multistream that referenced this issue Aug 6, 2019
Replace `MultiStream()` with `new MultiStream()` now. Readme examples have always shown this usage, but now it is required.

For: brave/brave-browser#5490
feross added a commit to webtorrent/ut_pex that referenced this issue Aug 6, 2019
feross added a commit to webtorrent/webtorrent that referenced this issue Aug 6, 2019
feross added a commit to webtorrent/webtorrent that referenced this issue Aug 6, 2019
feross added a commit to webtorrent/webtorrent that referenced this issue Aug 7, 2019
feross added a commit to random-access-storage/random-access-file that referenced this issue Aug 7, 2019
Use the new fs.mkdir {recursive: true} option instead of the mkdirp dependency.

It might be too early to merge this, since Node 8 is still supported until the end of the 2019. But just sending this PR so we can merge it at some point in the future.

For: brave/brave-browser#5490
feross added a commit to random-access-storage/random-access-file that referenced this issue Aug 7, 2019
Use the new fs.mkdir {recursive: true} option instead of the mkdirp dependency.

It might be too early to merge this, since Node 8 is still supported until the end of the 2019. But just sending this PR so we can merge it at some point in the future.

For: brave/brave-browser#5490
@feross
Copy link
Author

feross commented Aug 7, 2019

Making progress. Almost done with this effort.

Before:

$ npm ls --depth=99 --production | wc -l
     267

$ npm run size
   98338

After:

$ npm ls --depth=99 --production | wc -l
     223

$ npm run size
   84329

@guanzo
Copy link

guanzo commented Aug 7, 2019

Could you also update stream-to-blob-url? It still depends on "stream-to-blob": "^1.0.0"

@feross
Copy link
Author

feross commented Aug 7, 2019

@guanzo Done. Released stream-to-blob-url@3.

@guanzo
Copy link

guanzo commented Oct 19, 2019

Hi, need help finishing this?

@jimmywarting
Copy link

How about switching out simple-get for fetch?
NodeJS is planing on releasing fetch in core. it have already implemented lots of underlying dependencies to make it happen like the whatwg stream. so get ready for it?

simple-get is not so simple as it may seem behind the source.

All this in < 100 8888 lines of code.

https://pastebin.com/82ayvGKB

Mather of fact: it embeds builtin-status-codes, inherits, readable-stream, xtend buffer, ieee754
http-stream, base64-js resulting in a wooping 263 kb after running npx browserify simple-get.js > out.js that originally only included window.x = require('simple-get')

replace that with isomorphic-fetch or cross-fetch and you have decreased the bundle by 99.99% (now i'm only talking about simple-get and not about webtorrent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/webtorrent Label for webtorrent related issues QA/No release-notes/exclude
Projects
None yet
Development

No branches or pull requests

4 participants