RFC: absorb ember-concurrency-async and standardize around async functions

[machty]

This RFC proposes that:

1. ember-concurrency-async be merged into ember-concurrency core (similar to how we merged in ember-concurrency-decorators)
2. We introduce a new @taskFn decorator that behaves almost exactly the same as the @task decorator, but produces directly-callable "async" functions that have safe Task-like semantics
3. We bring in the taskFor helper from ember-concurrency-ts, but we extend its (currently no-op / typing-only) behavior to extract Task objects from @taskFn functions.
4. We introduce an async-arrow function equivalent to the above for use with task()

Background

ember-concurrency-async (ECA) is a complementary addon to ember-concurrency that allows you to use async functions instead of generator functions when defining EC tasks, e.g.

import { task } from 'ember-concurrency';

class Foo {
  @task({ restartable: true }) async foo() {
    await timeout(300);
    // ...
  }
}

The way this works is that ECA provides a babel transform to convert the async fn to a generator function (this is required because async fns are not externally cancellable, whereas generators can be used to model cancellable async functions).

This has a few benefits:

1. TypeScript's handling of async functions is much more robust than its much-lacking support of generators and the yield keyword.
2. An EC tasks behaves, in spirit, much more like an async fn than how generators are used (EC is a lot of people's first experience with generators, and lot of people have the same question as to "why don't we just use async fns")

There are some downsides:

1. The use of a babel transform is a bit magical; it bends the semantics of async fns just a bit which to the astute JS programmer can understandably cause some confusion
2. ECA still requires the well-typed async fns that perform() EC tasks to use await taskFor(this.myTask).perform() when what you really want to do is just type await this.myTask().

There isn't much we can do about #1 until/unless TS offers better support for generator fns (unlikely) or async fns become cancellable by way of some new EcmaScript spec (also unlikely).

Proposal: @taskFn decorator

To alleviate some of the awkwardness around #2 above, I propose we merge ECA into EC, and we introduce a new @taskFn decorator that can be used in place of any @task decorator, but instead of converting the decorator generator/async function to a .perform()-able task object with derived state properties, it "converts" it to an async function that can be directly invoked/called (rather than .perform()ed) with Task-like safety guarantees. For example:

import { safe } from 'ember-concurrency';

export default class extends Component {
  @taskFn async parentDoStuff(ms: number) {
    await timeout(ms);
    await this.childDoStuff();
    return 'done!';
  }

  @taskFn async childDoStuff() {
    // ...
  }
}

The @taskFn decorator will accept the same arguments that @task decorator takes today, e.g. you could write the following:

export default class extends Component {
  @taskFn({
    maxConcurrency: 3,
    restartable: true
  })
  async doStuff() {
    // ...
  }
}

With the @taskFn keyword, we can entirely avoid the taskFor() ceremony for performing tasks in a type-safe manner; instead of await taskFor(this.doStuff).perform() you can juse to await this.doStuff()

@taskFor helper

Because @taskFn async functions themselves are just Function instances installed on the prototype (or instance), they won't, by default, have any kind of "derived state" (like isRunning, etc) that we've come to know and love. In order to re-expose this derived state, I propose we bring in the @taskFor function from ember-concurrency-ts, which is currently a no-op fn used to appease TypeScript, and extend it just slightly so that you can pass it an async @taskFn and have it return to you the hidden/internal Task object (which you can then use to access derived state like .isRunning).

export default class extends Component {
  @taskFn async doStuff() {
    // ...
  }

  doStuffTask = taskFor(this.doStuff);
}

With this in place you can put the following in templates:

{{#if this.doStuffTask.isRunning}}
   wat
{{/if}}

With this scheme, there'd basically be two "entry points" to the same @taskFn async fn: 1. calling the fn directly, or 2. Calling .perform() on the Task object returned from taskFor.

async-arrow tasks

This is a more recent idea that emerged from discussions in the Discord e-concurrency channel (thank you @NullVoxPopuli!):

In addition to the above APIs which are somewhat async-function centric (i.e. the thing that gets installed on the host object is an async function with special task-y semantics), I propose we introduce a more class Task-centric API wherein the object that gets installed on the host object is a Task that you can .perform() -- this will always be my favorite kind of API because Tasks are so easy and self-contained to pass around and access the derived state of.

class DemoComponent {
  foo = 5;
  doStuffTask = task(this, async () => {
    await timeout(500);
    this.foo++;
  });
}

This is a well-typed TS component, with extremely minimal ceremony (with one bonus being you don't need to add types to this -- it knows that this is a DemoComponent).

To pass options:

class DemoComponent {
  foo = 5;
  doStuffTask = task(this, { restartable: true }, async () => {
    await timeout(500);
    this.foo++;
  });
}

To make this work, we would apply the same kind of transform to async arrow fns, which would turn the above into something like:

class DemoComponent {
  foo = 5;
  doStuffTask = task(this, { restartable: true }, () => {
    return function * () {
      yield timeout(500);
      this.foo++;
    }
  });
}

We might also want to make it possible to use this API in conjunction with the @use resources decorator, but really the only thing it would do is allow you to omit that first this arg that you have to pass to task():

class DemoComponent {
  foo = 5;
  @use doStuffTask = task({ restartable: true }, async () => {
    await timeout(500);
    this.foo++;
  });
}

I prefer we land the this-based version first since it relies less on Ember-specific machinery.

[NullVoxPopuli]

I love it. And especially default drop: true :sweat_smile:

[gnclmorais]

I like all of this, if anything I think we could try to brainstorm some alternatives to safe — this word means everything and nothing at the same time, so if we could find a somewhat more explicit replacement, even better! I’ll pitch a few: taskLike, fnLike, taskAsFn, simpleTask, plainTask, toFn, toFunction, callableTask. They might sound unpolished, but I’m just interested in getting the ball rolling…

[maxfierke]

To alleviate some of the awkwardness around #2 above, I propose we merge ECA into EC, and we introduce a new @safe decorator that can be used in place of any @task decorator, but instead of converting the decorator generator/async function to a .perform()-able task object with derived state properties, it "converts" it to an async function that can be directly invoked/called (rather than .perform()ed) with Task-like safety guarantees. For example:

Likewise, I think @safe is maybe a tricky word to use here. Perhaps something like @constrained? Or @statelessTask?

Having two very similar concepts, but having them be exclusively usable in two different context means we need to be very careful about making it easy to remember when you can use one and not the other. In that vain, I do still think there's an argument to be made about having our cake and eating it too (i.e. one @task that you apply to either generator functions or async functions) and make them callable either way (whether it's via babel/build-time transformation or the Function prototype hack to make them callable)

Because @safe async functions themselves are just Function instances installed on the prototype (or instance), they won't, by default, have any kind of "derived state" (like isRunning, etc) that we've come to know and love. In order to re-expose this derived state, I propose we introduce a @taskFor decorator that will "wrap" a @safe function and promote it to a full on classic Task object that can be .perform()ed and passed around to child components, etc, just like classic Task objects:

I like it. There might be a little roughness to migration for people already using taskFor that we might need to think about (Maybe we call it something different?) Would also think about adding a toTask() standalone helper or method to @safe-ified functions to make it possible to convert outside of a class definition

Mini-proposal: make drop: true be the default for @safe

I would argue we should just pull this out into its own thing and make it the case for @task (and whatever @safe-type thing) in 3.x. Seems fairly uncontroversial at this point (a good percentage of the time @task is used without constraints, its a bug). At the very least, we could provide something like @unconstrainedTask or @limitlessTask or @dangerousUnsafeYouProbablyDontWantThisTask or something.

[machty]

@gnclmorais Thanks for the suggestions. I'd want to avoid the names that sound like they're casting to a function from some other object, when the decorator is going to be decorating what is already a function and letting it remain a function (this is actually part of the reason why it's more TS friendly than classic tasks: decorators can't change the type of a field in current TS).

Just for kicks, I made an Ember Twiddle to test out new decorator names to see what looks ok: https://ember-twiddle.com/5ecbd214ab0af2d786701e4421b5a711?openFiles=templates.application%5C.hbs%2C

You didn't quite suggest this one but maybe @taskFn is OK? It mentions what it is without suggesting it's casting from something else.

(edit: deleted a chunk from an earlier draft I meant to delete)

[machty]

@maxfierke instead of @taskFor decorator could we overload @task decorator to accept a string to point to the task fn? e.g.:

export default class extends Component {
  @taskfn async doStuff() {
    // ...
  }

  @task('doStuff') doStuffTask;
}

[machty]

There's some potential perf wins when using only function-only tasks; we can disable some of the internal derived state tracking until/unless it's promoted to a full on task. Then again, is this something we can detect from the get go? If a Task object attaches to a task fn in a late-bound manner, we might have already disabled state-tracking by that point?

[maxfierke]

You didn't quite suggest this one but maybe @taskFn is OK? It mentions what it is without suggesting it's casting from something else.

I think @taskFn actually looks pretty good and then fn piece might be enough to provide hints as to when/where to use it.

There's some potential perf wins when using only function-only tasks; we can disable some of the internal derived state tracking until/unless it's promoted to a full on task. Then again, is this something we can detect from the get go? If a Task object attaches to a task fn in a late-bound manner, we might have already disabled state-tracking by that point?

I think its probably worth quantifying the perf questions. For example, if it's just the state tracking that's expensive, but not the Task piece, we can already disable state tracking by passing a null onState callback (i.e. @task({ onState: null }), which we could alias as @statelessTask or something for more convenience). I'm not sure if the late-binding thing is possible (it might be w/ the stage 1 decorator transform, but w/ the new decorator proposal updates, it might not be)

[gnclmorais]

I think @taskFn actually looks pretty good and then fn piece might be enough to provide hints as to when/where to use it.

Same, I like this proposal — thank you for coming up with it! 🎊

[machty]

I've updated/edited the proposal based on community feedback:

• Rename @safe decorator to @taskFn
• Replace proposed @taskFor decorator with an extended form of the taskFor() utility fn from ember-concurrency-ts

I haven't changed or addressed the mini proposal at the end "Mini-proposal: make drop: true be the default for @taskFn ". I'm leaning towards voting no on this, because it I want the upgrade path to be easy and I don't want people having to think/wonder whether a taskFn needs to be configured a certain way to behave as it did before.

[maxfierke]

This looks great! Count me as onboard. I cannot think of any big implementation blockers either, so it seems feasible as well.

I suspect we'll need to refactor some internal stuff around where the WeakMaps that hold Task instances live and how that all works generally, so that taskFor can pull from that (right now, they're stored in a closure around the property descriptor)

[machty]

Removed the mini-proposal to change behavior to drop.

One thing that came up in the Discord was that 1. @taskFn should almost certainly produce an async fn that is bound to the parent object, which also means 2. anyone that combines @taskFn with @action is going to be double-binding the fn; with @taskFn, there is no need to use @action. Just pointing this out for posterity.

I think this RFC is ready or implementation.

[machty]

@NullVoxPopuli supplied a code example in the #e-concurrency discord channel that knocked some of my thinking loose, and I've added another async-arrow-based variant of task() to this RFC. Honestly I like it more and will probably use it more for my app than the async-fn variants, not to mention it's probably less work to implement.

[acorncom]

I like the concepts getting kicked around here. One thing that occurs to me is that in an Embroider world where addons are supposed to compile down to act as importable libraries, will we easily have the ability to do Babel transforms once an app’s code isn’t transformable by the addon pipeline?

I also wonder if we’re going to see a future world where Babel increasingly becomes irrelevant as the JS world starts exploring other transformation tooling, so would hate to see us build a new set of APIs that lock us into an older set of tooling. But that’s probably further future-looking than the Embroider question …

[machty]

@acorncom In Embroider world, apps will need to manually register the ember-concurrency async arrow fn transform. There may be ways to used embroider macros or some other APIs to make sure the devex is good in this regard but I'm/we're not too worried about that aspect.

As far as other tooling goes: hard to predict this, but I think the current approach will be a safe bet for the foreseeable future. Perhaps by the time people move on from Babel transforms, decorators in TS will support changing types, and we may have the flexibility to return to generator functions.

[maxfierke]

@acorncom In Embroider world, apps will need to manually register the ember-concurrency async arrow fn transform. There may be ways to used embroider macros or some other APIs to make sure the devex is good in this regard but I'm/we're not too worried about that aspect.

thought I just had: how does this impact add-ons which use ember-concurrency? I'm assuming because in v2/Embroider world, they would be "built" that this wouldn't be a concern, but I'm not sure.

[acorncom]

@acorncom In Embroider world, apps will need to manually register the ember-concurrency async arrow fn transform. There may be ways to used embroider macros or some other APIs to make sure the devex is good in this regard but I'm/we're not too worried about that aspect.

thought I just had: how does this impact add-ons which use ember-concurrency? I'm assuming because in v2/Embroider world, they would be "built" that this wouldn't be a concern, but I'm not sure.

@maxfierke I was thinking about that case when the discussion first started as well. I think your assumption is accurate (regardless of whether an addon uses the newer async arrow fn transform or not, by the time addon code is compiled/published it will be in an easily consumable format that shouldn't need further transformation).

[machty]

465 implemented proposition 4, which I think is all we need at this point.