ember-concurrency icon indicating copy to clipboard operation
ember-concurrency copied to clipboard

RFC Proposal: Expose low-level coroutine functions

Open sukima opened this issue 9 years ago • 7 comments

Expose low-level coroutine functions

In the same vein of tj/co it would be helpful to use coroutine patterns in places outside of tasks.

Much of ember uses promises. Knowing that promises improve the state of asynchronous code, they also introduce a level of cognitive complexity that is difficult to maintain in many situations. e-c tasks and coroutines are interchangeable with promises.

This is a proposal to offer the same interchange functionality that e-c uses under the hood to manage tasks to the user so they can use them in places where a task would be inappropriate.

Possible use cases (non-exhaustive list)

  • Asynchronous test cases
  • beforeModel, model, and afterModel hooks
  • Custom networking services
  • Asynchronous procedures in build scripts
  • ember generate server code
  • Addon code (scripts used as part of blueprints and/or build hooks)

At the moment if you want to use coroutines in these cases you have to npm install co --save-dev which I think is node specific and not included in the final ember output.

Using other coroutine libraries is duplicating efforts since e-c already implements these under the hood.

Proposed API (A)

import { spawn } from 'ember-concurrency';

let promise1 = spawn(function * () {});  // => Promise
let wrapped = spawn.wrap(function * () {}); // => Function
let promise2 = wrapped();                // => Promise

Proposed API (B)

import { coroutine } from 'ember-concurrency';

let promise1 = coroutine(function * () {});    // => Promise
let wrapped = coroutine.wrap(function * () {}); // => Function
let promise2 = wrapped();                      // => Promise

Examples

test('…', spawn.wrap(function * (assert) {
  yield visit('…');
  yield clickOn('button');
  assert.ok($('button').is(':disabled'));
});
model: spawn.wrap(function * () {
  let data = yield $.ajax({url: '…'});
  let data2 = yield $.ajax({url: '…', data});
  return data2.map(itemData => MyItem.create(itemData));
})
model() {
  let users = spawn(function * () {
    let config = yield $.ajax({url: '/config'});
    return Ember.get(config, 'memberships.users');
  });

  let posts = $.ajax({url: '/posts'});

  return RSVP.hash({users, posts});
}
var spawn = require('ember-concurrency/spawn');

function wrap(gen) {
  let fn = spawn.wrap(gen);
  return function (req, res, next) {
    return fn(req, res, next).catch(next);
  };
}

module.exports = function (app) {
  app.get('/api/post', wrap(function * (req, res) {
    let data = {
      meta: { foo: 'bar' },
      data: yield getDataAsync()
    };
    res.setHeader('Content-Type', 'application.json');
    res.send(JSON.stringify(data));
  }));
};

sukima avatar Oct 06 '16 00:10 sukima

I like this idea — it fills a void for folks who for whatever reason can't use async/await yet, and it provides some nice sugar for spinning off smaller one-off tasks in the context of a larger one. A few questions around particulars:

  • Are the promises returned by spawn(...)/spawn.wrap(...)() task instances that can be canceled with the normal semantics around subtasks, etc? I would imagine so, with the likely exception that they're not owned by some Ember object with the automatic cleanup that entails.
  • Do spawn.wrapped generators have any of the concurrency control primitives (restartable and friends) of tasks? (Guessing no — for that complexity, just use a proper task.)
  • How do we think about documenting this? Presumably we want to discourage people from using spawn.wrap in places where task is possible (assuming no ownership, as mentioned above), but I can easily imagine people gravitating toward it for simple cases given the more ergonomic this.go() vs this.get('go').perform().

dfreeman avatar Oct 06 '16 14:10 dfreeman

some of my thoughts from #e-concurrency

machty [10:26 AM]
but it seems there's a big problem we should solve/avoid if we go this route

[10:27]
so 1) I don't want to implement something that just behaves the exact same as async/await but is just wrap(function *()) / yield

[10:27]
at some point, if the cancelation spec for async/await aligns properly, i'd love to just use async functions instead of function * (the core team prefers this too)

[10:27]
so if it just meant porting async/await to async, i'm not sure that makes sense

[10:27]
that said

[10:28]
because e-c needs cancelation and task modifiers, those necessitate function * syntax to give e-c the power to do what it needs to do

[10:28]
SO

[10:28]

Do spawn.wrapped generators have any of the concurrency control primitives (restartable and friends) of tasks? (Guessing no — for that complexity, just use a proper task.)

[10:28]
my gut instinct on this was "no", but i'm thinking about it more

[10:29]
one thing that's a smell about this proposal is that if you use .wrap() on an object that might be destroyed, we'd lose e-c's ability to cancel on object destruction

[10:29]
unless...

[10:30]
we could make e-c always check this.isDestroyed before resuming a .wrapped fn

[10:30]
additionally, if we were feeling wiley, we could make .wrap() install the same destroy listeners

[10:31]
i'd have to rearrange some things to support it

[10:33]
so the more i think about it, we could bring some of the niceties of task modifiers / auto-cancel to .wrap

[10:34]
@cfreeman / @cowboyd you might be interested in this stuff too

[10:34]
it's somewhat related to the decomplecting we were discussing

[10:35]
basically, I see a way to replace the current approach of using ComputedProperty subclasses to install a Task on the object, with an alternative approach that doesn't involve any CP / ember-metal magic

[10:36]
lemme write down some examples

machty [12:49 PM]
sorry for the delay, been discussing some internals w the core team

[12:49]
i'll start sharing some details

[12:50]
today you have to do this.get('myTask').perform(...) to perform a task

[12:50]
it'd be nice if we could do this.myTask(...)

[12:51]
unfortunately, this implies that myTask would be a function, and ember's stepping away from any solution that involves setting/binding to properties like isRunning and isIdle on instances of Function

[12:52]
so, .perform() is here to stay

[12:52]
but that leaves this.myTask.perform() as a target API

[12:55]
which I think is possible to support but i'll have to spike on it to see

[12:57]
but that's orthogonal to the question of standalone e-c coroutines

[12:59]
if we were to make .wrap() work, which essentially takes a task generator function and produces a function that returns a promise-y task instance, there is a way to still preserve all task semantics

[12:59]
including task modifiers and cancel-on-destroy

[12:59]
but since it'd be a function, there'd be no place to put/access .isRunning / .isIdle

[1:00](the task instances returned would have similar properties, but you wouldn't be able to check myTask.isRunning)

[1:00]
does that make sense? /cc @sukima @dfreeman

machty avatar Oct 12 '16 17:10 machty

We've been discussing this RFC in the e-concurrency slack channel, and it seems like consensus is building for the following:

.toFunction()

Instead of having a separate .wrap function / chained method, we could just supply a .toFunction() task modifier. Unlike the other task modifiers (e.g. .drop() / .restartable()), .toFunction() wouldn't just modify the TaskProperty descriptor but would actually produce an immediately-usable Function. This function could be callable on its own, or as a method of an object, e.g.

import { task, timeout } from 'ember-concurrency';

let fn = task(function * () {
  yield timeout(5000);
  // ...
  return 123;
}).toFunction();

fn().then((v) => {
  alert(v);
});

export default Ember.Component.extend({
  doStuff: fn,
});

The functions produced by .toFunction() share the same semantics as normal tasks, with the exception that there's no concept of "top-level" Task state, like .isRunning or .isIdle. The reason for this is that

  1. this single task function will live on the prototype and be shared by all instance of the host class, and 2) even if you produced a new function for each instance of the host class, a) writing properties to instances of Function is slow in most JS engines, b) Ember core itself is stepping away from this pattern, and perhaps most importantly c) ember-metal explicitly forbids binding to properties on Function.

Supporting this will involve some internal reorganization; right now, task state lives on the Task object the each instance of a class that declares task, e.g. if FooComponent declares myTask: task(...), the state that tracks whether myTask is running lives inside an instance of Task on an instance of FooComponent. In order to support task functions preserving the same concurrency semantics as plain ol task()s, we'll move this state to some shared, global cache keyed on (taskFunction, hostObject). I imagine there is a clever way to build this cache that leverages WeakMap if it exists, otherwise the cache can garbage collect hostObjects for which isDestroyed is true.

Concurrency semantics for task functions

let debouncedFn = task(function * () {
  yield timeout(500);
  console.log("Woot");
}).restartable().toFunction();

let obj0 = { debouncedFn };
let obj1 = { debouncedFn };

object0.debouncedFn(); // cancels immediately
object0.debouncedFn(); // cancels immediately
object0.debouncedFn(); // runs to completion

object1.debouncedFn(); // cancels immediately
object1.debouncedFn(); // cancels immediately
object1.debouncedFn(); // runs to completion

debouncedFn(); // cancels immediately
debouncedFn(); // cancels immediately
debouncedFn(); // runs to completion

The behavior of the above example is the same whether we're using POJOs or Ember.Objects. The interesting bit here is that we're using .restartable() here, which conceptually only makes sense when a task has an "owner". With task functions, the "owner" is whatever the this context is when you invoke the task function. This why in the above example, calling object1.debouncedFn() doesn't cancel the task instance returned from object0.debouncedFn(), but multiple calls to object0.debouncedFn() will cancel previous iterations. Functions called with a falsy context will share the same global context (chances are, in most cases, you'll apply task modifiers to task functions that live on objects as methods).

machty avatar Oct 12 '16 19:10 machty

Side note: putting a task function on an object's prototype means you can just write this.taskFn() instead of the classic e-c this.get('fooTask').perform(). I can't do anything about .perform() here, but I believe I can make this.fooTask.perform() work so as to minimize the semantic differences between task objects and task functions.

machty avatar Oct 12 '16 19:10 machty

Anyone interested in implementation progress on this RFC can follow along w the to-function branch: https://github.com/machty/ember-concurrency/tree/to-function

machty avatar Nov 16 '16 17:11 machty

This is very very exciting!

sukima avatar Nov 17 '16 13:11 sukima

For a work-around here is an alternative addon: https://www.npmjs.com/package/ember-co

sukima avatar Sep 16 '17 01:09 sukima