Add type definition based on API docs

[chancancode]

Yet another attempt at adding types.

Quite similar to #355, but this includes every item (and only those items) in the public API documentation. Also uses interfaces over classes, since the e-c classes are not meant to be instantiated manually or subclassed by apps.

[machty]

This lgtm to I'm behind on the TypeScript discussion / tradeoffs when it comes to e-c so I'll defer to others.

[chancancode]

@maxfierke I have added (mostly just copied) the docs for TaskGroup

[maxfierke]

@chancancode I think this looks great! Before I approve, are there any caveats you can think of that would be worth mentioning or potentially breaking for folks (outside of people using their own types)? I'm thinking specifically about TypeScript not supporting changes to the return types of generators at one point, IIRC.

[chancancode]

Before I approve, are there any caveats you can think of that would be worth mentioning or potentially breaking for folks (outside of people using their own types)?

These PR adds the following top level type symbols:

• TaskGenerator
• TaskFunction
• TaskFunctionArgs
• TaskFunctionReturnType
• Task
• TaskGroup
• TaskInstance
• TaskProperty
• TaskCancelation
• Yieldable
• Evented

These can be imported from the ember-concurrency module by name when using TypeScript, so they are effectively public API once released.

Because they are type symbols, non-TypeScript projects won't be able to import them, and TypeScript projects won't be able to do anything with them other than using them as types (e.g. you cannot do new Task() because that would require Task being a "value" symbol, but Task is just a type here (which is erased from the code once compiled).

These names are effectively "reserved". It would be okay if, for example, ember-concurrency wants to export the same, currently internal, Task class as a public API for subclassing. In that case, we will just update the TypeScript definition from an interface to a class, all the existing properties will match up just fine. However, if ember-concurrency wants to use the Task named export for something completely different (say, export cons Task = "Task, the string.";), that would not be okay since the type definition will be forced to find a different name for what is currently exported as Task, and that would be a breaking change.

In practice, I think this seems unlikely to be a problem since I was quite careful to pick the names to match the defacto public names in the documentation (@class Task, etc). It's also possible to reclaim these symbols for something else by bumping major version.

I'm thinking specifically about TypeScript not supporting changes to the return types of generators at one point, IIRC.

I think this is referring to the fact that the decorator cannot change the type of the property it's decorating. So since most people write @task *foo()... you cannot convince typescript to let you call this.foo.perform(...) no matter what.

This is not really an ember-concurrency problem since the decorators are outside of this repository. Based on the way I typed it, the classic style foo: task(function *() { ... }) should work _when used with this.get('foo').perform(...). I think this.foo.perform(...) still won't work there since the task function is typed to return a TaskProperty.

The workaround in our project (very similar to @NullVoxPopuli's) is the following utility functions:

// app/types/ember-concurrency.d.ts

import {
  Task,
  TaskFunction,
  TaskFunctionArgs,
  TaskFunctionReturnType,
  TaskInstance
} from 'ember-concurrency';

/**
 * No-op typecast function that turns what TypeScript believes to be a
 * generator function into a Task.
 *
 * ```js
 * import { taskFor } from 'direwolf/types/ember-concurrency';
 *
 * class Foo extends EmberObject {
 *   @task *myTask() {
 *     // ...
 *   }
 *
 *   someMethod() {
 *     this.myTask.perform(); // TypeError
 *     taskFor(this.myTask).perform(); // ok!
 *   }
 * }
 * ```
 *
 * @param task The task. Note that this is purely a typecast function,
 *   it does not in affect accept a task generator function as input.
 */
export function taskFor<T extends TaskFunction<any, any[]>>(task: T):
  Task<TaskFunctionReturnType<T>, TaskFunctionArgs<T>>;

// app/types/ember-concurrency.js

import { assert } from '@ember/debug';

export function taskFor(task) {
  assert(
    `${task} does not appear to be a task!`,
    task && typeof task.perform === 'function'
  );

  return task;
}

In actual usage:

// @ts-check
// app/components/foo.js

import { action } from '@ember/object';
import Component from '@glimmer/component';

import { task } from 'ember-concurrency-decorators';

import { taskFor } from 'my-app/types/ember-concurrency';

export default class extends Component {
  /**
   * @param {string} foo
   */
  @task *myTask(foo) {
    // ...
    return 1;
  }

  async someMethod() {
    this.myTask.perform("foo"); // error
    this.myTask.isRunning; // error

    taskFor(this.myTask).perform("foo"); // ok
    taskFor(this.myTask).perform(); // error
    taskFor(this.myTask).perform(123); // error

    let foo: number = await taskFor(this.myTask).perform("foo"); // ok
    let bar: string = await taskFor(this.myTask).perform("foo"); // error

    taskFor(this.myTask).isRunning; // ok

    let task = taskFor(this.myTask);

    task.perform("foo"); // ok
    task.perform(); // error
    task.perform(123); // error

    foo = await task.perform("foo"); // ok
    bar = await task.perform("foo"); // error

    task.isRunning; // ok
  }
}

[chancancode]

would you be willing to add a page to the docs w/ that example?

Sure, but I'll need a couple of pointers:

• Is there an inline way to write code snippets, or do I just make the file in the snippets folder?
• Does the "snippets" support .ts or .d.ts?
• Should I be using decorators in the example or using classic syntax? I suppose the latter could work, but it doesn't work super well with TypeScript, but perhaps it works good enough for the purpose of the example.
• Glimmer or classic component?
• For classic syntax at least, I believe .get is supposed to Just Works™ here, should I be using that?

would it make sense to include such a type-cast function as part of this?

I'm not sure. For one, it's kind of weird to have an empty function that doesn't do anything for non-TypeScript users. The other is that it's basically an "unsafe" function that works by lying to the compiler to mask the type error. I don't think it's particular dangerous, because at the end of the day it's just a weird quirk/fallout, but I'm not sure it's good to include it as an official API.

[maxfierke]

• Is there an inline way to write code snippets, or do I just make the file in the snippets folder?

Yep, you can do this in HBS:

{{! BEGIN-SNIPPET some-template-name }}

{{! END-SNIPPET }}

or in JS/TS

// BEGIN-SNIPPET

// END-SNIPPET

It uses https://github.com/ef4/ember-code-snippet underneath, so anything mentioned in the docs should apply within e-c (though, we use 2.4.x, so we do have syntax highlighting and do not have the get-code-snippet helper mentioned on the main page)

• Does the "snippets" support .ts or .d.ts?

Yep, it should.

• Should I be using decorators in the example or using classic syntax? I suppose the latter could work, but it doesn't work super well with TypeScript, but perhaps it works good enough for the purpose of the example.

Anecdotally, it seems most folks using TypeScript are using it with decorators, so we can probably stick to those.

• Glimmer or classic component?

I'm assuming both are fairly similar? Glimmer would probably be okay on its own, but if there are notable differences, it might be good to note those.

• For classic syntax at least, I believe .get is supposed to Just Works™ here, should I be using that?

Probably worth mentioning in the docs, but we should probably prefer using regular ES getters.

I don't think it's particular dangerous, because at the end of the day it's just a weird quirk/fallout, but I'm not sure it's good to include it as an official API. Got it. Sounds okay to leave out then.

[chancancode]

I'm writing some docs, but sounds like @chriskrycho wants to review this also

[maxfierke]

I'm writing some docs, but sounds like @chriskrycho wants to review this also

awesome! his suggestion of type tests would be super valuable too, if that's a thing

[chriskrycho]

As promised, Typed Ember RFC: Type Stability for Addons.

[chancancode]

Update: I have added the requested documentation, will work on the tests next. In the meantime please review what I wrote.

Rendered screenshot, for reference (the content will likely be outdated, but you can review the general formatting):

[chancancode]

Pushed a new commit to add the type tests, ember-try and CI config

[chancancode]

Rebased and updated to account for #358, I think this is good to go!

[chriskrycho]

Thank you so much! I have this on my list for the week—it will likely be Tuesday or Wednesday before I have time to review it in detail!