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

Does not work with native classes #271

Closed
mfeckie opened this issue Jan 16, 2019 · 17 comments
Closed

Does not work with native classes #271

mfeckie opened this issue Jan 16, 2019 · 17 comments
Labels
docs Issues related to documentation ember-octane Issues related to use with Ember "Octane" Edition

Comments

@mfeckie
Copy link

mfeckie commented Jan 16, 2019

Using Ember 3.6, ember-concurrency 0.8.26

Given a component defined with 'classic' syntax, all is good.

import Component from '@ember/component'
import { task, timeout } from 'ember-concurrency';

export default Component.extend({
  result: null,
  doStuff: task(function*(){
    yield timeout(1000);
    this.set('result', 'done');
  })
});

If, however, we use native class syntax

import Component from '@ember/component';
import { task, timeout } from 'ember-concurrency';

export default class NativeTaskComponent extends Component {
  result = null;
  doStuff = task(function*() {
    yield timeout(1000);
    this.set('result', new Date());
  });
}

We see -task-property.js:620 Uncaught Error: It looks like you tried to perform a task via this.nameOfTask.perform(), which isn't supported. Use 'this.get('nameOfTask').perform()' instead.

I've spent a little time trying to figure out why this is the case and have found that when creating a TaskProperty, the function passed to super does not get executed https://github.com/machty/ember-concurrency/blob/master/addon/-task-property.js#L416-L437

export class TaskProperty extends _ComputedProperty {
  constructor(taskFn) {
    let tp;
    console.log('called before super');
    super(function(_propertyName) {
      console.log('called inside super anonymous function');
      taskFn.displayName = `${_propertyName} (task)`;
...

The transpiled output of the classic invocation is

define("dummy/components/classic-task/component", ["exports", "ember-concurrency"], function (_exports, _emberConcurrency) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  var _default = Ember.Component.extend({
    result: null,
    doStuff: (0, _emberConcurrency.task)(function* () {
      yield (0, _emberConcurrency.timeout)(1000);
      this.set('result', 'done');
    })
  });

  _exports.default = _default;
});

Whilst the native version is

define("dummy/components/native-task/component", ["exports", "ember-concurrency"], function (_exports, _emberConcurrency) {
  "use strict";

  Object.defineProperty(_exports, "__esModule", {
    value: true
  });
  _exports.default = void 0;

  function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

  class NativeTaskComponent extends Ember.Component {
    constructor(...args) {
      super(...args);

      _defineProperty(this, "result", null);

      _defineProperty(this, "doStuff", (0, _emberConcurrency.task)(function* () {
        yield (0, _emberConcurrency.timeout)(1000);
        this.set('result', new Date());
      }));
    }

  }

  _exports.default = NativeTaskComponent;
});

So it looks like it's caused by the way the property ends up being defined on the native class that is causing the issue.

@buschtoens
Copy link
Contributor

Duplicate of #241

@buschtoens
Copy link
Contributor

The three solutions are:

  • define task in .extend({ }) block, like
    class extends Component.extend({
      myTask: task(function*() {}),
    }) {
      // rest of the class body
    }
  • use defineProperty, which has performance implications
  • use ember-concurrency-decorators, for which I need to update the type definitons

@mfeckie
Copy link
Author

mfeckie commented Jan 16, 2019

OK, perhaps it's worth updating the docs to reflect that, as this issue is likely to come up more frequently as the community marches forward

@machty
Copy link
Owner

machty commented Jan 16, 2019

Totally agree that this needs to be documented, as I always forget what the rules are myself. Would also appreciate a section in the doc on what the rules/options are for TypeScript. I'm very busy lately (and also a tad rusty) so I'd very much welcome a docs PR on this stuff.

@aaronfischer
Copy link

aaronfischer commented Jan 16, 2019

Hmm, I'm seeing the "classic" syntax generating that error: Class constructor ComputedProperty cannot be invoked without 'new':

  setupSso: task(function* () {
    let tokenValue = this.getTokenParam();
    if (tokenValue) {
    ...

References (-task-property.js:398):

  function TaskProperty(taskFn) {
    let tp = this;
    _utils._ComputedProperty.call(this, function (_propertyName) {     // THIS LINE
      taskFn.displayName = `${_propertyName} (task)`;
      return Task.create({
        fn: tp.taskFn,

@crhayes
Copy link

crhayes commented Jan 17, 2019

@aaronbhansen I am experiencing the same thing after moving from Ember 3.5 --> Ember 3.6. During this process I was upgraded from Babel 6 --> Babel 7. Could this be part of the issue?

@aaronfischer
Copy link

@crhayes After some random shots in the dark, I made sure that ember-concurrency was running X.X.26, which required upgrading other dependencies (like ember-power-select and an internal add-on we maintain), did this looking within yarn.lock.
This "seems" to have resolved the issue, I haven't been able to fully test yet, but it did remove the error on application load.

@buschtoens
Copy link
Contributor

Pretty sure you are running into #262 and #265, which is why making sure you are on the latest version of ember-concurrency fixed it. 😊

@crhayes
Copy link

crhayes commented Jan 17, 2019

@buschtoens Thanks! I realize (after checking my package.json) that it was hardcoded to 0.8.20.

I have an in-repo-addon that also uses ember-concurrency. I had to also add add "ember-concurrency": "^0.8.26" as a dependency of the in-repo-addon (in it's package.json).

@buschtoens
Copy link
Contributor

buschtoens commented Apr 1, 2019

For completeness sake: In the Octane edition of Ember.js, you can create a task like this:

class MyClass {
  @task(function*() {
    // ...
  }) myTask;
}

@gabrielgrant
Copy link

@buschtoens i think you're maybe missing a closing paren?

class MyClass {
  @task(function*() {
    // ...
  }) myTask;  // <-- note closing paren on this line
}

@buschtoens
Copy link
Contributor

@gabrielgrant That is correct, thanks! I updated the snippet. :)

@BryanCrotaz
Copy link
Contributor

BryanCrotaz commented Aug 2, 2019

What’s the correct syntax for a task using typescript with ember 3.11?

@buschtoens
Copy link
Contributor

@BryanCrotaz The issue is that decorators cannot (yet) influence the type signature. To circumvent this, I made ember-concurrency-typescript, but it's not fully done yet: buschtoens/ember-concurrency-typescript#6

@maxfierke maxfierke added docs Issues related to documentation ember-octane Issues related to use with Ember "Octane" Edition labels Aug 8, 2019
@jenweber
Copy link

jenweber commented Nov 1, 2019

Could we start by adding an example to the Task Function Syntax page? Or do we also need to cover the caveats/edge cases for it to be merged?

Example gist: https://gist.github.com/jenweber/443dac9876c7ef2b1115093cfd5d6fac

@buschtoens
Copy link
Contributor

I'd support adding the @task(function*() {}) syntax to the official docs, as @jenweber suggested.

We can also mention ember-concurrency-decorators as an alternative. Related: #326

maxfierke added a commit that referenced this issue Nov 25, 2019
Raised in issue #271, there is currently no official documentation
on using ember-concurrency with native classes and other Octane-related
use-cases.

This adds a brief section to "Task Function Syntax" in the docs that
shows an example of use with a Glimmer component and mentions the
ember-concurrency-decorators addon as a more concise approach to the
default decorators via the task() property.
maxfierke added a commit that referenced this issue Nov 25, 2019
Raised in issue #271, there is currently no official documentation
on using ember-concurrency with native classes and other Octane-related
use-cases.

This adds a brief section to "Task Function Syntax" in the docs that
shows an example of use with a Glimmer component and mentions the
ember-concurrency-decorators addon as a more concise approach to the
default decorators via the task() property.
@maxfierke
Copy link
Collaborator

Closing this out, since documentation has been added on using with native classes on Ember 3.10+. More specific issues w/ native classes, decorators, etc. can be addressed in separate issues as needed in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues related to documentation ember-octane Issues related to use with Ember "Octane" Edition
Projects
None yet
Development

No branches or pull requests

9 participants