Getting rid of `.perform()` in JS

[buschtoens]

tl;dr

// I want
this.someTask('🥦');
this.someTask.performCount; // => 1

// instead of
this.someTask.perform('🥦');
this.someTask.performCount; // => 1

---

We already took the first step

This nifty trick allows users to just use the {{action}} helper with tasks in their templates:

https://github.com/machty/ember-concurrency/blob/5c830b1dfdc9609b4de841b503bdb36023bbb4b1/addon/-task-property.js#L395-L397

You'd use it as:

<button onclick={{action deleteUser user}}>
  Delete {{user.name}}
</button>

This is super sexy. 🔥

The templates are agnostic to whether or not the thing that you're passing is a regular (closure) action or a task.

I would love to have the equivalent in JS and get rid of .perform().

It's already improved in JS

Since ES5 getters landed, ergonomics already improved massively for tasks, because instead of this:

export default Ember.Controller.extend({
  someTask: task(function*() {
    console.log('😐');
  }),
  
  actions: {
    performSomeTask() {
      this.get('someTask').perform();
    }
  }
});

You can know just go (Ember Twiddle):

export default Ember.Controller.extend({
  someTask: task(function*() {
    console.log('😍');
  }),
  
  actions: {
    performSomeTask() {
      this.someTask.perform();
    }
  }
});

One last step

But what's still annoying is that, when you refactor a simple method to a task, you have to update all the places that call this method, because you need to switch from:

this.someMethod('🥦');

to:

this.someMethod.perform('🥦');

What would it take?

The TaskProperty, when it is .get()ed should not return an object that is a Task, but instead should return a function that has all the Task stuff on its prototype (?). There might be some mad science involved, but assuming that

a) this does not hurt performance b) we do not remove .perform() (for now) to avoid API churn c) it works reliably

would you be open to this change? Then I might try my hand at a PR and see if this even works.

[maxfierke]

I know there was a discussion about this on #e-concurrency, but totally forgot what was discussed, and now it's lost to Slack history ¯\(ツ)/¯

But: I find this compelling and would love to see a PoC! (The performance question is top of mind for me here though)

[buschtoens]

The core problem was that the Ember observer / computed property system does not watch properties on functions. This means that someTask.performCount would be broken as someTask is a function.

If we changed that in Ember core, then we could do this. 😄

[sandstrom]

Has there been any discussions on moving ember-concurrency into Ember itself?

If we can use the action helper in the template, and possibly get rid of perform, there isn't much of a difference between actions/methods and tasks from an API perspective.

Does anyone know if the Ember core team would be open to it? Ember-concurrency could live on as an addon, for experimentation about task pipelines, task grouping, derived states and other things.

[alexander-alvarez]

@sandstrom https://twitter.com/amatchneer/status/1045710076584112130

[EndangeredMassa]

@buschtoens do you know if Ember will watch function properties with Octane?

[buschtoens]

@EndangeredMassa Looks like it!

import Component from "@glimmer/component";
import { tracked } from "@glimmer/tracking";
import { action, computed, defineProperty } from "@ember/object";
import { reads } from "@ember/object/computed";

export default class FooComponent extends Component {
  @tracked i = 1;

  get iTimes2() {
    return this.i * 2;
  }

  @action
  increment() {
    this.i++;
  }

  someTask = function() {};

  constructor(...args) {
    super(...args);

    this.someTask.host = this;
    defineProperty(this.someTask, "readsAlias", reads("host.iTimes2"));

    Object.defineProperty(this.someTask, "times4", {
      get() {
        return this.host.iTimes2 * 2;
      }
    });
  }
}

[buschtoens]

I will burn in hell for this 😂

import Component from "@glimmer/component";
import { defineProperty } from "@ember/object";
import { reads } from "@ember/object/computed";
import { task as ecTask, timeout } from "ember-concurrency";

export default class FooComponent extends Component {
  @task
  someTask = function*() {
    yield timeout(1000);
    return Math.random();
  };
}

const PROXY_KEYS = ["performCount", "last", "isRunning"];

function task(Class, taskName, descriptor) {
  return {
    ...descriptor,
    initializer() {
      const taskFn = descriptor.initializer.call(this);
      const taskProp = ecTask(taskFn);
      const realTaskName = `__${taskName}`;
      defineProperty(this, realTaskName, taskProp);

      const perform = function(...args) {
        return this[realTaskName].perform(...args);
      }.bind(this);

      perform.host = this;

      for (const key of PROXY_KEYS) {
        defineProperty(perform, key, reads(`host.${realTaskName}.${key}`));
      }

      return perform;
    }
  };
}

[buschtoens]

Alright, actually got it working without any hackery:

import Component from "@glimmer/component";
import { defineProperty, notifyPropertyChange } from "@ember/object";
import { reads } from "@ember/object/computed";
import { timeout } from "ember-concurrency";
import { Task, TaskProperty } from "ember-concurrency/-task-property";
import { TaskGroup } from "ember-concurrency/-task-group";
import { resolveScheduler } from "ember-concurrency/-property-modifiers-mixin";

export default class FooComponent extends Component {
  // This _has_ to be the class field syntax, since the Babel legacy transforms don't support
  // decorating generator methods. 😢
  @task
  someTask = function*() {
    yield timeout(1000);
    return Math.random();
  };
}

function task(Class, taskName, descriptor) {
  return {
    ...descriptor,
    initializer() {
      // @TODO: support concurrency modifiers, like `restartable`, via additional decorators.
      const tp = new TaskProperty();
      tp.maxConcurrency(1);

      const taskFn = descriptor.initializer.call(this);

      // e-c currently requires a new `Task` instance per host object instance. In a future
      // version we would reuse the same task and store host object specific state in a WeakMap.
      const task = Task.create({
        fn: taskFn,
        context: this,
        _origin: this,
        _taskGroupPath: tp._taskGroupPath,
        _scheduler: resolveScheduler(tp, this, TaskGroup),
        _propertyName: taskName,
        _debug: tp._debug,
        _hasEnabledEvents: tp._hasEnabledEvents
      });

      const perform = (...args) => {
        const ti = task.perform(...args);

        // Since the e-c code accesses these properties indirectly and not through the host
        // object, the tracking system does not pick up on the changes. By acessing the props
        // via the host object and sending updates, we can work around that.
        // @see https://github.com/machty/ember-concurrency/blob/62c942d7f41e1555ab835ba921f1293f0c8d1984/addon/-scheduler.js#L95-L97
        notifyPropertyChange(this[taskName], "numRunning");
        notifyPropertyChange(this[taskName], "numQueued");
        ti._onFinalize(() => {
          notifyPropertyChange(this[taskName], "numRunning");
          notifyPropertyChange(this[taskName], "numQueued");
        });

        return ti;
      };
      Object.setPrototypeOf(perform, task);

      return perform;
    }
  };
}