-
-
Notifications
You must be signed in to change notification settings - Fork 287
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
Switch from downloadjs to multi-download #174
Merged
Merged
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
6fed0b5
Delete FUNDING.yml
blenderskool 2494dea
Merge pull request #157 from blenderskool/next
blenderskool 60e9185
fix: docker-compose now
ruoduan-hub f8acd96
Merge pull request #161 from ruoduan-hub/master
blenderskool d9cbb0c
Merge pull request #170 from blenderskool/next
blenderskool 0e6f609
Migrate to GA4
blenderskool 8b9e588
Switch from downloadjs to multi-download
ddelange 72de2fe
Move to utils/download.js
ddelange abe585a
Amend comment
ddelange 9cd46a8
Add URL.revokeObjectURL
ddelange b11cee7
Simplify delay behaviour, add JSDoc comments
ddelange 89ba327
Typo
ddelange 6f5c806
Fix typo
ddelange 993e47b
Fix multi-file downloads for WebTorrent
blenderskool a6ae197
Update client/src/utils/download.js
blenderskool File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// adapted from https://github.com/sindresorhus/multi-download/blob/v4.0.0/index.js | ||
// to take File as input https://developer.mozilla.org/en-US/docs/Web/API/File | ||
const delay = milliseconds => new Promise(resolve => { | ||
setTimeout(resolve, milliseconds); | ||
}); | ||
|
||
const download = async (file) => { | ||
const a = document.createElement('a'); | ||
a.download = file.name; | ||
a.href = URL.createObjectURL(file); | ||
a.style.display = 'none'; | ||
document.body.append(a); | ||
a.click(); | ||
|
||
// Chrome requires the timeout | ||
await delay(100); | ||
a.remove(); | ||
}; | ||
|
||
const multiDownload = async (files) => { | ||
if (!files) { | ||
throw new Error('`files` required'); | ||
} | ||
|
||
for (const [index, file] of files.entries()) { | ||
await delay(index * 1000); // eslint-disable-line no-await-in-loop | ||
download(file); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, it looks like
File.getBlob
was removed in webtorrent v2.this seems related to them switching to local storage in v2 as opposed to keeping complete torrents in memory in v1 ref webtorrent/webtorrent#86 (comment)
edit: they now have
File.blob
to load the whole torrent into memory and (more interestingly)File.streamURL
(requiresclient.createServer
to be ran beforehand).Maybe blaze can even get rid of
download.js
altogether using the local storage functionality of webtorrent v2 🤔they usethis PR was superceded againFileSystemDirectoryHandle
ref webtorrent/webtorrent#2208There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blaze currently uses Webtorrent v1, and we'll need to upgrade to v2. From what I can recall since the last time I had checked, there were a bunch of API changes associated with v2 that we'd also have to make in Blaze which I wasn't fond of picking up then. Would you be interested in picking this up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I'm also afraid it's a can of worms... the local storage feature might make it worthwhile though. I guess a 10GB file would now fill up 10GB of my swap right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
judging by the File API docs, I think using streamURL you can initiate a download before the torrent has finished downloading, and so download.js could theoretically be called upon as soon as the torrent is initiated.
most importantly, that way the file doesn't need to go fully into memory in a Blob in order to send to downloads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file.getBlob and file.blob are functionally identical, it was simply renamed to match the W3C file API, rather than create our own structure, but getBlob also loaded the entire file into memory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah just saw your cross comment, so
file.streamURL
is a viable option to get around the 'whole file in memory' pattern in the current downloading setup, with minimal overhead fromclient.createServer
. that's great to hear and probably the way to move forward there!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddelange isn't the local storage limited to like 10MBs? I wonder how it would manage files of much higher sizes in such a limited space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in v2 the torrents directly download to
/tmp/webtorrent
by default using file system access.path
can be set on a per-torrent basis by the receiver before the torrent is added (folder selection is google chrome only if I understood the comments correctly)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the sake of simplicity lets say sure, but if you want specifics, it actually writes to individual chunks via FSA's origin private file system, which functionally is no different from the likes of localStorage or IDB, except its a LOT faster, and allows for MUCH, MUCH, MUCH bigger file sizes, in extreme cases up to terabytes
on chrome that's a bit different, because IF YOU CHOOSE TO specify a rootDir, it can emulate the file structure in a specified folder as already mentioned, on top of saving chunks as cache to OPFS
that's the big difference for webtorrent v2, the storage and interfaces it provides:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for pointing to rootDir!
api.md points to storeOpts.rootDir but I think that should be opts.rootDir right?
based on the comments in api.md, I'm now not sure what's the difference between opts.path and opts.rootDir. should I ignore opts.path and only work with opts.rootDir?