Skip to content
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

Lightweight Decorators #42

Closed
gossi opened this issue Apr 7, 2019 · 3 comments
Closed

Lightweight Decorators #42

gossi opened this issue Apr 7, 2019 · 3 comments

Comments

@gossi
Copy link

gossi commented Apr 7, 2019

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
Copy link
Collaborator

buschtoens commented Apr 24, 2019

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 babel/babel#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 microsoft/TypeScript#2983 would allow 💯 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
Copy link
Collaborator

Closing in favor of #41.

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

@gossi
Copy link
Author

gossi commented Apr 24, 2019

👍 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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants