RFC: macro-style task modifiers

[machty]

trafficstars

What if, instead of / in addition to .drop()/.restartable()/.enqueue() task modifiers, we exposed yieldable functions that essentially act as macros that you put at the beginning of the task fn. Example:

    import { task, restartable, cancelPrior } from 'ember-concurrency';

    // classic
    restartableTask: task(function * () {
      yield doAsyncStuff();
    }).restartable(),

    // macro style
    restartableTask: task(function * () {
      yield cancelPrior();

      yield doAsyncStuff();
    }),

I'd imagine the following kinds of names:

• Instead of .restartable(), you do yield cancelPrior()
• Instead of .drop(), you do yield cancelIfAlreadyRunning()`
• Instead of .enqueue(), you do yield enqueue()`

or we just keep it simple and keep the names (yield drop() / yield restartable() / yield enqueue()).

Either way, benefits include:

1. Visually moving the "task modifier" to the top of the task, instead of having it at the bottom, beneath a bunch of code
2. You'd have more runtime control over whether these macros fired, e.g. maybe you only want to cancel prior tasks instances in certain cases, but if you just mark as .restartable(), you've opted out of such control, but with macros you could do:

    myTask: task(function * (isImportant) {
      if (isImportant) {
        yield cancelPrior();
      } else {
        yield enqueue();
      }

      doAsyncStuff();
    }),

If these look like the kind of guards / defensive programming that EC is supposed to replace, keep in mind it's just a single line of code vs a chained task modifier, and just as before, swapping "task modifiers" is just as easy as changing a single word.

3. If you do <form onsubmit={{perform submitForm}}, and submitForm is .drop(), then there's no opportunity to event.preventDefault() and the browser ends up performing a form submission and reloading/redirecting the page. The hackish way around this right now is using the old-school element style action helper e,g, <form {{action (perform...)}} which auto-preventDefaults(); alternatively you can write a (prevent-default) helper that preventDefault()s before calling your action/task). But if you used the proposed macro style of task modifiers, you could do

    submitForm: task(function * (event) {
      event.preventDefault();
      yield drop();
      // asyncStuff
    }),

4. With the upcoming "yieldables" API, it would be possible to define custom "task modifiers" using this approach. I've been considering exposing the API for building your own custom drop/enqueue etc., but it seems weird that it'd be its own separate API, whereas with the macro approach, everything's a Yieldable, and all special task logic can live / be configured with such an API.

[cibernox]

I like the approach.

There is a benefit not listed here that probably is not a big deal, but this approach with explicit imports is friendly to treeshaking, whereas having all the modifiers attached to TaskInstances prevents it.

I want to suggest other names for those functions. Those modifiers that cancel (drop, restartable) tasks can start with cancel* and those that put the task on hold (enqueue) can wait*

• drop -> cancelIfRunning
• restartable -> cancelPrevious
• enqueue -> waitPrevious

I also see an opportunity to simplify of the API of the component.

The maxConcurrency option can be replaced by the user if there was a way of knowing the number of running tasks (I assume there is, I just don't know it so I'm going to invent an API here)

doFoo: task(function*() {
  if (this.get('doFoo.activeTasks.length') === 3) {
    yield drop();
  }
});

I'm not sure how you'd obtain the number of running tasks in the upcoming coroutines tho.

Also, keepLatest could be implemented in userland with this

keepLatestFoo: task(function*() {
  if (this.get('doFoo.enqueuedTasks.length') > 0) {
    yield cancelPrevious(); // This would cancel the enqueued one, not the running one.
  }
  yield waitPrevious(); // enqueues this task
});

Whether or not this reduction in the amount of magic is justified, I'm not sure. But I'd be OK with writing a simple if in exchange of self-explainatory code.

[machty]

RE: treeshaking, figured I'd mentione we could also make the classic drop/restartable/enqueues separate tree-shakeable imports, either by using the decorators API (which I explored at one point and didn't like), or doing something like myTask: restartable(task(...)) but I don't like that either. It's nice that the macro API is pleasant enough AND gets tree-shaking benefits.

Thanks for the feedback! Most/all of this is implementable on the "perf" branch, gonna whip up an example real quick.

[patcoll]

The API feels more Ember-y with a syntax like myTask: restartable(task(...)). If I could +1 any design it would be that, keeping the existing wording as much as possible. It just feels more functional and consistent with current Ember best practices.

That said, it does seem useful to be able to opt-in to a syntax to use inside task code itself that would make handling certain async situations more ergonomic.

Ideally, both would be possible, with the functional style being the most straight-forward migration path if the old syntax was deprecated.

[machty]

Here's an experimental twiddle demonstrating cancelPrevious and cancelIfRunning

https://ember-twiddle.com/27166d7befc73c7a927844ca9896bc5d?numColumns=2&openFiles=controllers.application.js%2Ctemplates.application.hbs