search

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

`rawTimeout()` does not work with `race()` #329

Closed Turbo87 closed 4 years ago

Turbo87 commented 4 years ago 
let result = yield race([someOtherPromise, rawTimeout(5000)]);

I've tried the above code, but it seems like the race() function is not considering the return value of rawTimeout() as something that can be waited upon. Using timeout() works fine though, so now I'm wondering if we should adjust the implementation of rawTimeout() to roughly match the one from timeout() 🤔

/cc @maxfierke @machty

maxfierke commented 4 years ago

@Turbo87 good catch, I think you're right:

https://github.com/machty/ember-concurrency/blob/3d96d8490c13220b23f105e3d535ccbd55d7534c/addon/-cancelable-promise-helpers.js#L123-L126

The cancelable promise helpers only seem to take into account those too. I think there's a built-in assumption that the cancelable promise helpers would be passed promise-ified task instances (which always have an __ec_cancel__) or TaskInstances, but that doesn't hold true for the more loosely defined "yieldables" that folks would probably expect to work here.

This is probably what's happening in #308 too.

Changing the implementation to make it similar to timeout so that it has an __ec_cancel__ is a quick fix for rawTimeout, but there's probably some other refactoring that needs to happen to work with the WaitFor-derived yieldables too. I'll take a look at this tonight, but if you get a PR up for the rawTimeout stuff before then, I'd merge that.

maxfierke commented 4 years ago

Closed by #331