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

Encapsulated task decorator #15

Closed willviles closed 5 years ago

willviles commented 6 years ago

Just wondering if there's any planned support (or workaround) for encapsulated tasks?

The following raises the error:

import { task } from 'ember-concurrency-decorators'
import { task as inlineTask } from 'ember-concurrency'

class Foo {
  @task({
    drop: true
  })
  fooTask = inlineTask({
    * perform () {
      // Task logic
    }
  })
}

Assertion Failed: ember-concurrency-decorators: Can only decorate a generator function as a task.

Perhaps the following syntax could work for a new decorator?

import { encapsulatedTask } from 'ember-concurrency-decorators'

class Foo {
  @encapsulatedTask({
    drop: true
  })
  fooTask = {
    * perform () {
      // Task logic
    }
  }
}
buschtoens commented 6 years ago

We definitely should support encapsulated tasks. Thank you for raising the issue!

In order to reduce the amount of different decorators, ideally all existing @task decorators would also be usable for encapsulated tasks. We can use the different syntactical constructs to differentiate between regular and encapsulated tasks, as you suggested.

import { dropTask } from 'ember-concurrency-decorators'

class Foo {
  @dropTask
  regularDropTask = function*() {
    ...
  };

  @dropTask
  encapsulatedDropTask = {
    somePrivateState: true,
    *perform() {
      ...
    }
  };
}

Implementation should be pretty straightforward. We just change this assertion to allow objects with a perform generator method on them as well.

https://github.com/machty/ember-concurrency-decorators/blob/fcd36840a3c07163eff1cf066e398a2630f865f6/addon/index.js#L58-L61

What do you think?

I am also open to alternative API suggestions.

We could potentially add an @encapsulate decorator (for lack of a better name) instead like:

import { dropTask, encapsulate } from 'ember-concurrency-decorators'

class Foo {
  @dropTask
  regularDropTask = function*() {
    ...
  };

  @dropTask
  @encapsulate({
    somePrivateState: true
  })
  encapsulatedDropTask = function*() {
    ...
  };
}

The reason being, that this could potentially look nicer with the soon supported generator method syntax:

import { dropTask, encapsulate } from 'ember-concurrency-decorators'

class Foo {
  @dropTask
  *regularDropTask() {
    ...
  }

  @dropTask
  @encapsulate({
    somePrivateState: true
  })
  *encapsulatedDropTask() {
    ...
  }
}
willviles commented 6 years ago

Wow, super speedy PR! Thanks @buschtoens.

I think it's probably overkill to introduce a secondary decorator. Just supplying an object is definitely the cleanest solution.