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
Remove timer dependency from event-helpers (listen, listenOnce) #8472
Remove timer dependency from event-helpers (listen, listenOnce) #8472
Conversation
ff6623b
to
2b78971
Compare
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.
LGTM, but couple of q
if (unlistenLoad) { | ||
unlistenLoad(); | ||
} | ||
failedToLoad(eleOrWindow); |
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.
failedToLoad
throws an exception, right? Not a huge fun of those methods. Could you please add a comment here and say that this method will rethrow the original error?
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.
Fixed.
* @param {!EventTarget} element | ||
* @param {string} eventType | ||
* @param {boolean=} opt_capture | ||
* @param {number=} opt_timeout | ||
* @param {function(!UnlistenDef)=} opt_cancel An optional function that, when |
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.
I suppose this is ok since this is relatively rare. But WDYT about cancellable promises? E.g. we could have a type extension with method cancel
on the promise?
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.
I thought about doing this, but it would be exposing it in the wrong object. Ie, how promise do not have a #resolve
or #reject
, only the resolver gets access to them. If we had a deferred type, we could easily add it there:
/**
* @typedef {{
* resolve: function(*=),
* reject: function(*=),
* cancel: function(),
* promise: !Promise,
* }}
*/
let DeferredDef;
4decc53
to
7fda6a5
Compare
It’s unused
This inverts the responsibility of calling the unlistener. Now, the caller can be given access the `unlisten` by passing a closure. The closure is called with `unlisten` as the param, giving the caller access. When the caller determines it is time to stop listening (due to timeout, etc), it can manually cleanup.
7fda6a5
to
d9ec050
Compare
497c5b2
to
021f606
Compare
…roject#8472) * Remove opt_timeout from loadPromise It’s unused * Allow caller to call internal unlisten This inverts the responsibility of calling the unlistener. Now, the caller can be given access the `unlisten` by passing a closure. The closure is called with `unlisten` as the param, giving the caller access. When the caller determines it is time to stop listening (due to timeout, etc), it can manually cleanup. * lint * fix merge conflict * fix test * Fix tests
…roject#8472) * Remove opt_timeout from loadPromise It’s unused * Allow caller to call internal unlisten This inverts the responsibility of calling the unlistener. Now, the caller can be given access the `unlisten` by passing a closure. The closure is called with `unlisten` as the param, giving the caller access. When the caller determines it is time to stop listening (due to timeout, etc), it can manually cleanup. * lint * fix merge conflict * fix test * Fix tests
…roject#8472) * Remove opt_timeout from loadPromise It’s unused * Allow caller to call internal unlisten This inverts the responsibility of calling the unlistener. Now, the caller can be given access the `unlisten` by passing a closure. The closure is called with `unlisten` as the param, giving the caller access. When the caller determines it is time to stop listening (due to timeout, etc), it can manually cleanup. * lint * fix merge conflict * fix test * Fix tests
This inverts the responsibility of calling the unlistener. Now, the
caller can be given access the
unlisten
by passing a closure. Theclosure is called with
unlisten
as the param, giving the calleraccess. When the caller determines it is time to stop listening (due to
timeout, etc), it can manually cleanup.
Fixes #8316.