Closed fregante closed 5 years ago
Agreed. I think it should be enabled by default. The main use-case of this module is getting an element before the dom is ready.
// If cancelled, return null like a regular querySelector() would
I think it should just return the cancel error, like when it's .cancel()
'ed, for consistency. Alternatively, make .cancel()
also return null
, but not sure about that. Could also not reject and just document that users needs to check for null
when the promise resolves.
Yeah, that RGH code expected an error so () => null
is in the catch
, but it could just resolve(null)
instead of cancelling.
const promise = new PCancelable((onCancel, resolve) => {
let raf;
onCancel(() => {
cancelAnimationFrame(raf);
});
+ domLoaded.then(() => {
+ cancelAnimationFrame(raf);
+ resolve(null);
+ });
// Interval to keep checking for it to come into the DOM
(function check() {
const el = options.target.querySelector(selector);
if (el) {
resolve(el);
selectorCache.delete(selector);
} else {
raf = requestAnimationFrame(check);
}
})();
});
@bfred-it But then I think .cancel()
should also resolve with null
, so consumers only have one place to handle "no result". What do you think?
If domLoaded
resolves to null
I have no opinion on how .cancel()
should behave 👍
I think this should be optional, since sometimes the use case is to use it when an element is inserted dynamically to the DOM (after documentReady was fired)
@liady Yes, there will be an option to not stop on dom-ready.
There should also be an option to stop listening after a period of time. I was looking at this for use in a UI testing library and it won't be good if test is broken and never finishes because the element never appears.
@issuehunt has funded $40.00 to this issue.
I was looking at this recently and I'm still stuck on how it can be solved.
until: Promise | false
option that defaults to domLoaded
? So it can also be set to:
elementReady('body', {
until: false
});
elementReady('body', {
until: delay(1000)
});
elementReady('body', {
until: Promise.race([
delay(1000),
domLoaded,
])
});
I would split it into two options.
How about this?
cancelOnDomLoaded: true
- Cancel if the element was not found by the time the DOM was loaded.timeout: Infinity
- Time to wait before giving up. The timeout starts from after the DOM has loaded.Not sure whether Infinity
is a good default.
Having a separate timeout
sounds reasonable, but all that is bit verbose and less flexible than a "cancel on promise"
I can implement cancelOnDomLoaded
if you want
👍
I moved the timeout option to https://github.com/sindresorhus/element-ready/issues/19
@sindresorhus has rewarded $36.00 to @bfred-it. See it on IssueHunt
IssueHunt Summary
#### [ bfred-it](https://issuehunt.io/u/bfred-it) has been rewarded. ### Sponsors (Total: $40.00) - [ issuehunt](https://issuehunt.io/u/issuehunt) ($40.00) ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/sindresorhus/element-ready/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.