-
-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: PoC for TypeScript type support #56
Changes from 1 commit
05cbb1a
2ce5abb
66830d7
5abc1d5
05fe952
8ec57cb
dc550f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
declare module 'ember-concurrency' { | ||
import EmberObject from '@ember/object'; | ||
import { | ||
UnwrapComputedPropertyGetter, | ||
UnwrapComputedPropertyGetters | ||
} from '@ember/object/-private/types'; | ||
|
||
import RSVP from 'rsvp'; | ||
|
||
// Lifted from @types/ember__object/observable.d.ts | ||
interface Getter { | ||
/** | ||
* Retrieves the value of a property from the object. | ||
*/ | ||
get<K extends keyof this>(key: K): UnwrapComputedPropertyGetter<this[K]>; | ||
/** | ||
* To get the values of multiple properties at once, call `getProperties` | ||
* with a list of strings or an array: | ||
*/ | ||
getProperties<K extends keyof this>( | ||
list: K[] | ||
): Pick<UnwrapComputedPropertyGetters<this>, K>; | ||
getProperties<K extends keyof this>( | ||
...list: K[] | ||
): Pick<UnwrapComputedPropertyGetters<this>, K>; | ||
} | ||
|
||
export type GeneratorFn<Args extends any[] = any[], R = any> = ( | ||
...args: Args | ||
) => IterableIterator<R>; | ||
|
||
export const all: typeof Promise.all; | ||
export const allSettled: typeof RSVP.allSettled; | ||
export const hash: typeof RSVP.hash; | ||
export const race: typeof Promise.race; | ||
|
||
export function timeout(ms: number): Promise<void>; | ||
|
||
export function waitForEvent( | ||
object: EmberObject | EventTarget, | ||
eventName: string | ||
): Promise<Event>; | ||
|
||
export function waitForProperty<T extends object, K extends keyof T>( | ||
object: T, | ||
key: K, | ||
callbackOrValue: (value: T[K]) => boolean | ||
): Promise<void>; | ||
|
||
export function waitForQueue(queueName: string): Promise<void>; | ||
|
||
export function task<Args extends any[], R>( | ||
taskFn: GeneratorFn<Args, R> | ||
): Task<Args, Exclude<R, Promise<any>>>; | ||
export function task<Args extends any[], R>(encapsulatedTask: { | ||
perform: GeneratorFn<Args, R>; | ||
}): Task<Args, Exclude<R, Promise<any>>>; | ||
|
||
export function taskGroup(): TaskGroupProperty; | ||
|
||
interface CommonTaskProperty { | ||
restartable: () => TaskProperty; | ||
drop: () => TaskProperty; | ||
keepLatest: () => TaskProperty; | ||
enqueue: () => TaskProperty; | ||
maxConcurrency: (n: number) => TaskProperty; | ||
cancelOn: (eventName: string) => TaskProperty; | ||
group: (groupName: string) => TaskProperty; | ||
} | ||
|
||
export interface TaskProperty extends CommonTaskProperty { | ||
evented: () => TaskProperty; | ||
debug: () => TaskProperty; | ||
on: (eventName: string) => TaskProperty; | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-empty-interface | ||
export interface TaskGroupProperty extends CommonTaskProperty {} | ||
|
||
// Based on https://github.com/CenterForOpenScience/ember-osf-web/blob/7933316efae805e00723789809bdeb58a96a286a/types/ember-concurrency/index.d.ts | ||
|
||
export enum TaskInstanceState { | ||
Dropped = 'dropped', | ||
Canceled = 'canceled', | ||
Finished = 'finished', | ||
Running = 'running', | ||
Waiting = 'waiting' | ||
} | ||
|
||
export interface TaskInstance<T> extends PromiseLike<T>, Getter { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may want to tweak this to be a union type where interface TaskInstanceBase<T> extends PromiseLike<T>, Getter {
readonly hasStarted: boolean;
readonly isCanceled: boolean;
readonly isDropped: boolean;
readonly isFinished: boolean;
readonly isRunning: boolean;
readonly isSuccessful: boolean;
readonly value?: T;
cancel(): void;
catch(): RSVP.Promise<unknown>;
finally(): RSVP.Promise<unknown>;
then<TResult1 = T, TResult2 = never>(
onfulfilled?:
| ((value: T) => TResult1 | RSVP.Promise<TResult1>)
| undefined
| null,
onrejected?:
| ((reason: any) => TResult2 | PromiseLike<TResult2>)
| undefined
| null
): RSVP.Promise<TResult1 | TResult2>;
}
interface InstanceDropped {
readonly state: TaskInstanceState.Dropped;
hasStarted: true; // ?
isCanceled: true; // ?
isDropped: true;
isError: false; // ?
isFinished: true; // ?
isRunning: false; // ?
isSuccessful: false; // ?
}
interface InstanceCanceled {
readonly state: TaskInstanceState.Canceled;
hasStarted: true; // ?
isCanceled: true;
isDropped: false; // ?
isError: false; // ?
isFinished: true; // ?
isRunning: false; // ?
isSuccessful: false; // ?
};
// etc.
interface InstanceError {
isError: true;
error: unknown;
hasStarted: true; // ?
isCanceled: false; // ?
isDropped: false; // ?
isFinished: true;
isRunning: false;
isSuccessful: false;
}
interface Success {
isError: false;
error: undefined;
hasStarted: true; // ?
isCanceled: false; // ?
isDropped: false; // ?
isError: false;
isFinished: true;
isRunning: false;
isSuccessful: true;
}
export type TaskInstance<T> = TaskInstanceBase<T> & (
| InstanceDropped
| InstanceCanceled
| InstanceFinished
| InstanceRunning
| InstanceWaiting
| InstanceError
| InstanceSuccess
); The |
||
readonly error?: unknown; | ||
readonly hasStarted: boolean; | ||
readonly isCanceled: boolean; | ||
readonly isDropped: boolean; | ||
readonly isError: boolean; | ||
readonly isFinished: boolean; | ||
readonly isRunning: boolean; | ||
readonly isSuccessful: boolean; | ||
readonly state: TaskInstanceState; | ||
readonly value?: T; | ||
cancel(): void; | ||
catch(): RSVP.Promise<unknown>; | ||
finally(): RSVP.Promise<unknown>; | ||
then<TResult1 = T, TResult2 = never>( | ||
onfulfilled?: | ||
| ((value: T) => TResult1 | RSVP.Promise<TResult1>) | ||
| undefined | ||
| null, | ||
onrejected?: | ||
| ((reason: any) => TResult2 | PromiseLike<TResult2>) | ||
| undefined | ||
| null | ||
): RSVP.Promise<TResult1 | TResult2>; | ||
} | ||
|
||
export enum TaskState { | ||
Running = 'running', | ||
Queued = 'queued', | ||
Idle = 'idle' | ||
} | ||
|
||
export interface Task<Args extends any[], T> extends Getter { | ||
readonly isIdle: boolean; | ||
readonly isQueued: boolean; | ||
readonly isRunning: boolean; | ||
readonly last?: TaskInstance<T>; | ||
readonly lastCanceled?: TaskInstance<T>; | ||
readonly lastComplete?: TaskInstance<T>; | ||
readonly lastErrored?: TaskInstance<T>; | ||
readonly lastIncomplete?: TaskInstance<T>; | ||
readonly lastPerformed?: TaskInstance<T>; | ||
readonly lastRunning?: TaskInstance<T>; | ||
readonly lastSuccessful?: TaskInstance<T>; | ||
readonly performCount: number; | ||
readonly state: TaskState; | ||
perform(...args: Args): TaskInstance<T>; | ||
cancelAll(): void; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,7 +8,9 @@ import { | |||||||||||||||||||||||||||||||||||||||
task as createTaskProperty, | ||||||||||||||||||||||||||||||||||||||||
taskGroup as createTaskGroupProperty, | ||||||||||||||||||||||||||||||||||||||||
TaskProperty, | ||||||||||||||||||||||||||||||||||||||||
TaskGroupProperty | ||||||||||||||||||||||||||||||||||||||||
TaskGroupProperty, | ||||||||||||||||||||||||||||||||||||||||
Task, | ||||||||||||||||||||||||||||||||||||||||
GeneratorFn | ||||||||||||||||||||||||||||||||||||||||
} from 'ember-concurrency'; | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
export { default as lastValue } from './last-value'; | ||||||||||||||||||||||||||||||||||||||||
|
@@ -86,7 +88,7 @@ function createTaskFromDescriptor(desc: DecoratorDescriptor) { | |||||||||||||||||||||||||||||||||||||||
(typeof value === 'object' && typeof value.perform === 'function') | ||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
return createTaskProperty(value); | ||||||||||||||||||||||||||||||||||||||||
return (createTaskProperty(value) as unknown) as TaskProperty; | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||
|
@@ -110,14 +112,15 @@ function createTaskGroupFromDescriptor(_desc: DecoratorDescriptor) { | |||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||
function applyOptions( | ||||||||||||||||||||||||||||||||||||||||
options: TaskGroupOptions, | ||||||||||||||||||||||||||||||||||||||||
task: TaskGroupProperty | ||||||||||||||||||||||||||||||||||||||||
taskProperty: TaskGroupProperty | ||||||||||||||||||||||||||||||||||||||||
): TaskGroupProperty & Decorator; | ||||||||||||||||||||||||||||||||||||||||
function applyOptions( | ||||||||||||||||||||||||||||||||||||||||
options: TaskOptions, | ||||||||||||||||||||||||||||||||||||||||
task: TaskProperty | ||||||||||||||||||||||||||||||||||||||||
taskProperty: TaskProperty | ||||||||||||||||||||||||||||||||||||||||
): TaskProperty & Decorator { | ||||||||||||||||||||||||||||||||||||||||
return Object.entries(options).reduce( | ||||||||||||||||||||||||||||||||||||||||
( | ||||||||||||||||||||||||||||||||||||||||
// eslint-disable-next-line no-shadow | ||||||||||||||||||||||||||||||||||||||||
taskProperty, | ||||||||||||||||||||||||||||||||||||||||
[key, value]: [ | ||||||||||||||||||||||||||||||||||||||||
keyof typeof options, | ||||||||||||||||||||||||||||||||||||||||
|
@@ -135,7 +138,7 @@ function applyOptions( | |||||||||||||||||||||||||||||||||||||||
value | ||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||
task | ||||||||||||||||||||||||||||||||||||||||
taskProperty | ||||||||||||||||||||||||||||||||||||||||
// The CP decorator gets executed in `createDecorator` | ||||||||||||||||||||||||||||||||||||||||
) as TaskProperty & Decorator; | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
@@ -192,7 +195,38 @@ const createDecorator = ( | |||||||||||||||||||||||||||||||||||||||
* @param {object?} [options={}] | ||||||||||||||||||||||||||||||||||||||||
* @return {TaskProperty} | ||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||
export const task = createDecorator(createTaskFromDescriptor); | ||||||||||||||||||||||||||||||||||||||||
const taskDecorator = createDecorator(createTaskFromDescriptor); | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
export function task<Args extends any[], R>( | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could use docs, of course! I think I'm following this; why does it require both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Unfortunately, either cannot do what the other one does, so we have to use this awkward double. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, that's what I thought; how do we make it not incur runtime costs? Or is the judgment simply that they're small enough—one extra generator function allocation, it looks like?—that it doesn't really matter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not allocating an extra generator function, luckily! :)
ember-concurrency-decorators/addon/index.ts Lines 219 to 226 in 66830d7
This means that in this case, @task
foo = task(function*() {});
// is equivalent to
@task
foo = function*() {}; This doesn't mean though, that we can't optimize this further with a Babel transform. ⚡️ 😄 @task
foo = task(function*() {});
// may be turned into
@task // from `ember-concurrency-decorators`
*foo() {}
// or even the following to avoid _any_ extra e-c-d runtime
@task(function*() {}) foo; // from `ember-concurrency` There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect rambling hereI need to correct myself. The following still does allocate a new generator function for every class instance: @task
foo = function*() {}; // takes the `desc.initializer` path While this one does not allocate a new generator function for every class instance: @task
foo*() {} // takes the `desc.value` path ember-concurrency-decorators/addon/index.ts Lines 63 to 73 in 5abc1d5
I think. tbh I'm not 💯sure right now, when in the life cycle Edit: No, looks like |
||||||||||||||||||||||||||||||||||||||||
taskFn: GeneratorFn<Args, R> | ||||||||||||||||||||||||||||||||||||||||
): Task<Args, Exclude<R, Promise<any>>>; | ||||||||||||||||||||||||||||||||||||||||
export function task<Args extends any[], R>(encapsulatedTask: { | ||||||||||||||||||||||||||||||||||||||||
perform: GeneratorFn<Args, R>; | ||||||||||||||||||||||||||||||||||||||||
}): Task<Args, Exclude<R, Promise<any>>>; | ||||||||||||||||||||||||||||||||||||||||
export function task(options: TaskOptions): PropertyDecorator; | ||||||||||||||||||||||||||||||||||||||||
export function task( | ||||||||||||||||||||||||||||||||||||||||
target: Record<string, any>, | ||||||||||||||||||||||||||||||||||||||||
propertyKey: string | symbol | ||||||||||||||||||||||||||||||||||||||||
): void; | ||||||||||||||||||||||||||||||||||||||||
export function task<Args extends any[], R>( | ||||||||||||||||||||||||||||||||||||||||
...args: | ||||||||||||||||||||||||||||||||||||||||
| [GeneratorFn<Args, R>] | ||||||||||||||||||||||||||||||||||||||||
| [{ perform: GeneratorFn<Args, R> }] | ||||||||||||||||||||||||||||||||||||||||
| [TaskOptions] | ||||||||||||||||||||||||||||||||||||||||
| [Record<string, any>, string | symbol] | ||||||||||||||||||||||||||||||||||||||||
): Task<Args, Exclude<R, Promise<any>>> | PropertyDecorator | void { | ||||||||||||||||||||||||||||||||||||||||
const [firstParam] = args; | ||||||||||||||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||||||||||||||
typeof firstParam === 'function' || | ||||||||||||||||||||||||||||||||||||||||
(typeof firstParam === 'object' && | ||||||||||||||||||||||||||||||||||||||||
// @ts-ignore | ||||||||||||||||||||||||||||||||||||||||
typeof firstParam.perform === 'function') | ||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||
// @ts-ignore | ||||||||||||||||||||||||||||||||||||||||
return firstParam; | ||||||||||||||||||||||||||||||||||||||||
// @ts-ignore | ||||||||||||||||||||||||||||||||||||||||
return taskDecorator(...args); | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||
* Turns the decorated generator function into a task and applies the | ||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With TypeScript 3.6, this becomes much clearer / safer.
https://devblogs.microsoft.com/typescript/announcing-typescript-3-6-beta/#stricter-generators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm so jazzed about that.
We should talk about TS version support at some point as well; it's probably fine to start with 3.6, given that and depending on when this lands and TS 3.6 lands, but the Typed Ember team is also working on hashing out a good ecosystem-wide versioning strategy as well to deal with the semi-regularity (and non-semver-compatible) breaking changes. Our current actual practice is "at least two most recent versions" but that's not going to work long-term.