machty / ember-concurrency-decorators

Task decorators for ember-concurrency 1.x (merged into ember-concurrency 2.0)
MIT License
70 stars 25 forks source link

Lightweight Decorators #42

Closed gossi closed 5 years ago

gossi commented 5 years ago

Since latest Octane builds, this is something possible at the moment:

Bildschirmfoto 2019-04-07 um 10 44 08

Reference: https://github.com/gossi/spomoda/blob/master/web/src/ui/routes/sport/manage/route.ts given these typings: https://github.com/gossi/spomoda/blob/master/web/types/ember-concurrency/index.d.ts @task is from e-c -> import { task } from 'ember-concurrency';

It has nice intelli-sense for auto-completion. The downside is, it only checks for type Function as an argument for @task() and not GeneratorFunction (that went problematic on the first try but may be solvable). It would increase type-safety, to not miss on the * here.

While that looks ok and nice, any modifications to that task, given chaining yields errors in your IDE, e.g. Property 'drop' does not exist on type 'PropertyDecorator'. for this one:

Bildschirmfoto 2019-04-07 um 10 45 51

So, we maybe can provide decorators on top, like so:

// restartable
@restartable
@task(function* () { 

}) myTask!: Task;

// restartable with options
@restartable({maxConcurrency: 4})
@task(function* () { 

}) myTask!: Task;

// just options
@taskOptions({maxConcurrency: 4})
@task(function* () { 

}) myTask!: Task;

Basic idea here: have @taskOptions as generic decorator, that takes all the options and then special ones, e.g. @restartable or @dropable that extend @taskOptions (as it is today).

Relates to #30 #35 #41

buschtoens commented 5 years ago

We had a discussion about the API design in #7. I think the long-term goal (think "static decorators" landed / TypeScript properly supports decorators) is still aligned with.

Have a generic @task decorator that accepts a hash of options plus some extra decorators that apply the common concurrency modifiers. So basically the API that we have right now.

@task({ restartable: true, maxConcurrency: 2 })
*foo() {}

// is equal to

@restartableTask({ maxConcurrency: 2 })
*foo() {}

Unfortunately, because of this bug https://github.com/babel/babel/issues/9852, we currently have to use the assignment syntax:

@task({ restartable: true, maxConcurrency: 2 })
foo = function*() {};

// is equal to

@restartableTask({ maxConcurrency: 2 })
foo = function*() {};

When this bug is fixed, we can allow the previous syntax again.

However, for TypeScript consumers, this still won't fly and IMO the best compromise solution is:

@task({ restartable: true, maxConcurrency: 2 })
foo = task(function*() {});

// is equal to

@restartableTask({ maxConcurrency: 2 })
foo = task(function*() {});

Here's why:

Based on some work I did in #35, this allows us to reliably extract the params, so that we can type check them in .perform(). It also allows us to extract the return type with a high level of certainty. Fixing https://github.com/Microsoft/TypeScript/issues/2983 would allow :100: return type extraction.

This syntax is also really close to the eventual long-term goal syntax. The decorators stay exactly the same and we just need to execute the following conversion:

@restartableTask({ maxConcurrency: 2 })
foo = task(function*() {});

// becomes

@restartableTask({ maxConcurrency: 2 })
*foo() {}

This is easily code modable.

buschtoens commented 5 years ago

Closing in favor of #41.

Let me know, if you disagree and we can reopen this.

gossi commented 5 years ago

👍 absolutely. I should have mentioned, the lightweight decorators might be something as an interim solution for the time being (while still codemodable? - I didn't checked that).