ember-native-dom-helpers
ember-native-dom-helpers copied to clipboard
cibernox
Simplify `waitUntil`
Was working on migrating this to @ember/test-helpers starting with the initial implementation here and simplified it a bit.
We could also probably remove the setTimeout(tick, 10) in the root of the promise body (by replacing with tick() directly), but it didn't seem much better (and guaranteeing a new stack seemed good to me).
Failing this assertion: https://github.com/cibernox/ember-native-dom-helpers/blob/6300642eb033c12631fe83b6b22c70594f6dff4c/tests/integration/wait-until-test.js#L113
This would be fixed by invoking tick() at the end of the promise callback body, but TBH I think the assertion should likely be removed. The only reason that it passes now (reliably anyways) is that Ember.RSVP is configured to resolve in the run-loop so things don't always get a next tick...
That immediate resolution was added in #97 to avoid the 10ms delay for checking something that might already be true. I don't have strong feelings, but I think that it makes sense to test the condition immediately before doing any waiting.
Did you do it for solving a problem?
IIRC the main problem I was trying to solve in that mentioned PR was that before that it would immediately resolve the promise but still schedule another tick timeout, leading to a situation that this tick call could be run when the test was already finished and teared down (because of the immediately resolving promise), leading to an exception (no DOM available any more) afterwards.
So the main assertion that I think is important there is that the callback is called just once: https://github.com/cibernox/ember-native-dom-helpers/blob/6300642eb033c12631fe83b6b22c70594f6dff4c/tests/integration/wait-until-test.js#L115
Wether the promise resolves immediately (when the condition is fulfilled) or after a first timeout is not important to me. I also think I would make sense to immediately resolve, but I have no strong feelings about that either.
I currently have https://github.com/rwjblue/ember-test-helpers/blob/b86cd963d6ff383be634d2f76a49bdbbd184578c/addon-test-support/%40ember/test-helpers/wait-until.js#L9-L28 in the WIP PR over in ember-test-helpers that I have been working on.
At least in ember-test-helpers its pretty important that the helper is always async (having APIs that are sometimes sync and sometimes async is super error prone), in the current WIP (link above) I am running tick function immediately (basically setTimeout(tick, 0)) which results in minimal delay when the condition is true, but still guarantees things are async.
I'll continue iterating over in that PR (though I would absolutely love the continued help reviewing), and once things have settled implementation wise I'll make another crack at syncing the two libs back up...