RFC: Public Yieldables API

[maxfierke]

Rendered

Implementation is included as well, which shows refactor of built-in Yieldables to use the new API. Refactoring (or similar) will probably stand, regardless of whether to promote to public API, as calling taskInstance.proceed with various magic values in a bunch of places is error prone.

[machty]

Looks great! Glad you're making this more public. I like that some of the ugly internals for "resuming" within the yieldable are hidden.

One thing though:

In general, Yieldable instances should not be reused across calls

I would change this so that you can resume a yieldable instance across calls. A Yieldable should be more like a stateless "description" of the async operation you want to build when e-c processes the Yieldable you yield to it. This is the basic idea behind Rx Observables: the observable objects don't have any state on them until you .subscribe() to it, and it allows for patterns of composability that are pretty neat (i.e. transform/combine a bunch of observables into one single observable that you subscribe to). I'm not sure those same composability benefits would come to the Yieldables API in the same way as it does for observables, but it seems like a better foundation to build on then one where yieldables can't be reused between yields.

From what I gather, I don't think making this change would actually change the public API for building yieldables, and it has the nice benefit that you would be able to reuse yieldable instances across calls, e.g.

const sharedTimeout = timeout(500);

// ...

yield sharedTimeout;
// do stuff
yield sharedTimeout;

vs having to wrap sharedTimeout in a function.

[maxfierke]

Looks great! Glad you're making this more public. I like that some of the ugly internals for "resuming" within the yieldable are hidden.

One thing though:

In general, Yieldable instances should not be reused across calls

I would change this so that you can resume a yieldable instance across calls. [...]

From what I gather, I don't think making this change would actually change the public API for building yieldables, and it has the nice benefit that you would be able to reuse yieldable instances across calls, e.g.

const sharedTimeout = timeout(500);

// ...

yield sharedTimeout;
// do stuff
yield sharedTimeout;

vs having to wrap sharedTimeout in a function.

@machty I think I might have phrased this weirdly... it is safe to use across calls within the same TaskInstance execution, but not across different TaskInstances. i.e. yes, you can make one call to timeout and yield that multiple times within the same task instance, but you could not, say, do the following:

const myTimeout = timeout(500);

class MyClass { 
  @task *aTask() {
    yield myTimeout
  }

  @task *bTask() {
     yield myTimeout;
   }
}

^ This would not be allowed.

If we want to allow that, I think we can do that, although the API might need to be a little bit different, especially to accomidate Promise casting of yieldables (which I just realized I forgot to define.)

[MLeganes]

Hallo there,

I am getting a lots of emails from you guys. This is because I was ussing your packed in my app when I was working in my company and now I am now working anymore and I would like to offer myself to help you. I have some experience with js and ember.js but I will need to learn a bit about concurrency. Will you need my help? Just let me know if it possible to help.

Best regards, MLeganes

[maxfierke]

I am getting a lots of emails from you guys.

@MLeganes it looks like you're "watching" the repo and thus are probably getting notifications for everything. If this is not what you want, you can unsubscribe by clicking "unwatch" at the top of the page or changing your notifications in that dropdown, or see this GitHub support article: https://docs.github.com/en/github/managing-subscriptions-and-notifications-on-github/configuring-notifications

[maxfierke]

@machty Revised the RFC last night a little bit in order to introduce YieldableState to keep the TaskInstance invocation state separate from the Yieldable instance's state. Would be curious if you have any additional thoughts?

Otherwise I think I'll probably move forward with this approach.