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

Adds timeout option to requestIdleCallback #1615

Merged
merged 1 commit into from Sep 7, 2019

Conversation

@guanzo
Copy link
Contributor

guanzo commented Apr 17, 2019

This is a modification of a previous pull request: #1513

Downloads will sometimes stall for several seconds, then resume after I click on the page, or blur the current window, or in any way cause requestIdleCallback to trigger. In some cases, the callback will never trigger if I reload the page and don't interact with it.

Delayed downloads are fine for streaming media since the user can consume the beginning chunks while later chunks get throttled, but for static media like images which need to render ASAP, the delayed download is counter productive.

I propose adding the timeout option which will guarantee that the callback runs in a reasonable time, while still allowing some idle time.

Note that the MDN docs recommend a timeout.

A timeout option is strongly recommended for required work, as otherwise it's possible multiple seconds will elapse before the callback is fired.

@Fenny

This comment has been minimized.

Copy link

Fenny commented May 15, 2019

Review please

@feross
feross approved these changes Sep 7, 2019
Copy link
Member

feross left a comment

This seems important. Thanks!

@feross feross merged commit e5a3a91 into webtorrent:master Sep 7, 2019
3 checks passed
3 checks passed
WIP Ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@feross

This comment has been minimized.

Copy link
Member

feross commented Sep 7, 2019

0.107.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.