Add tasks.js module #3839
Add tasks.js module #3839
Conversation
src/js/timer.js
Outdated
| // https://github.com/behnammodi/polyfill/blob/1a5965edc0e2eaf8e6d87902cc719462e2a889fb/window.polyfill.js | ||
|
|
||
| timer.requestIdleCallback = function (callback, options) { | ||
| options = options || {}; |
There was a problem hiding this comment.
@gorhill I'm not sure that all of this is useful but I've left it as it is. You had a much simpler polyfill. Let me know if you'd like to use that one.
src/js/static-net-filtering.js
Outdated
| @@ -26,6 +26,7 @@ | |||
| /******************************************************************************/ | |||
|
|
|||
| import globals from './globals.js'; | |||
| import timer from './timer.js'; | |||
There was a problem hiding this comment.
For now only this file uses it, but once all files use it then we can fix globals.js.
src/js/timer.js
Outdated
| const needsPolyfill = timer.requestIdleCallback === null; | ||
|
|
||
| if ( needsPolyfill ) { | ||
| // https://github.com/behnammodi/polyfill/blob/1a5965edc0e2eaf8e6d87902cc719462e2a889fb/window.polyfill.js |
There was a problem hiding this comment.
The code below is from the polyfill linked from here: https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback
There was a problem hiding this comment.
I prefer to keep the implementation simple -- I had a look at this polyfill back when I created globals but decided against it because there was no use for uBO of more complicated than what is there, the use-case in uBO is simply to defer some operations so as to not delay readiness at launch time. I prefer your idea of a common variable allowing access to common utility functions throughout different parts of the API. For instance requestIdleCallback() could simply be a custom-named method using requestIdleCallback() underneath, but without having all the semantic of requestIdleCallback().
There was a problem hiding this comment.
What about a simple API like this?
let id = timer.queue(myFunc);
timer.drop(id);timer.drop() is not even necessary for now.
static-net-filtering.js does pass a timeout value so I'm wondering if that should be supported.
There was a problem hiding this comment.
We could use a default timeout internally without requiring it to be part of the API -- the MDN doc strongly suggests the use of a timeout to avoid cases where the callback may never be called.
There was a problem hiding this comment.
I've renamed the file to tasks.js and it's a lot simpler now.
There was a problem hiding this comment.
I prefer your idea of a
commonvariable allowing access to common utility functions throughout different parts of the API.
We could call this file common.js if you prefer it. Afterwards, when there's a solution for location, that could go in this file too.
| @@ -3691,18 +3692,18 @@ FilterContainer.prototype.freeze = function() { | |||
| // work properly, so defer this until later to allow for reduced delay to | |||
| // readiness when no valid selfie is available. | |||
| if ( this.optimizeTimerId === undefined ) { | |||
| this.optimizeTimerId = globals.requestIdleCallback(( ) => { | |||
Related discussion: - #3839 (comment)
Related discussion: - #3839 `requestIdleCallback` can be assumed always present on browser-related platforms.
This patch adds a
tasks.jsfile.For now it's used only in
static-net-filtering.js.