machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
690 stars 157 forks source link

Implement `animationFrame()` yieldable #365

Closed Turbo87 closed 4 years ago

Turbo87 commented 4 years ago

This PR implements a animationFrame() utility that wraps the native requestAnimationFrame() API and can be used via yield animationFrame() (similar to yield timeout(1000 / 60)).

Related: https://github.com/machty/ember-concurrency/issues/169

Turbo87 commented 4 years ago

Could you add a quick test?

any suggestions on how to test this? 😅

maxfierke commented 4 years ago

Could add a unit test that stubs out rAF (sinon should be available in the test suite) and asserts that the correct things are called and that the task runs to completion without error (might not even need to stub out rAF)

Adding to these existing tests too would be good:

Turbo87 commented 4 years ago

@maxfierke I've added some tests but I fail to understand why the TS typing tests are failing. They seem to work alright locally.

@chancancode any clues?

Turbo87 commented 4 years ago

@maxfierke @chancancode any ideas how to proceed? the failing tests seem unrelated to the code that I introduced here 🤔

maxfierke commented 4 years ago

Looks like those typescript tests are also failing in https://github.com/machty/ember-concurrency/pull/368. I'll take a look at it this week and try and figure it out

maxfierke commented 4 years ago

@Turbo87 master's passing now, if you want to give this a rebase?

Turbo87 commented 4 years ago

hmm, seems there is still an unrelated test failing. or maybe it's just flaky. can you restart the failed test to check?

maxfierke commented 4 years ago

yeah, that's just a flaky one. I queued up a restart on that failed job but doesn't seem to be taking. I'll merge as-is