New Rule: no-async-actions

[Turbo87]

see http://ember-concurrency.com/docs/tutorial (scroll down to Version 4)

Bad:

actions: {
  async handleClick() {
    // ...
  }
}

actions: {
  handleClick() {
    return something.then(() => { /* ... */ });
  }
}

@action
async handleClick() {
  // ...
}

@action
handleClick() {
  return something.then(() => { /* ... */ });
}

Good:

actions: {
  handleClick() {
    return nothingOrSomethingThatIsNotAPromise;
  }
}

[rajasegar]

@Turbo87 For the decorators, should we need to configure some parser options i am getting the following error. Any help would be appreciated.

[Turbo87]

good question, I don't know 😅

we might have to configure the babel-eslint parser for those cases

[rajasegar]

@Turbo87 I need your help in formulating the docs for this rule
Is it on the same lines with eslint/no-async-promise-executor rule in eslint
https://github.com/eslint/eslint/blob/master/docs/rules/no-async-promise-executor.md

@bmish needs some clarifications here
#424 (comment)

[Turbo87]

Is it on the same lines with eslint/no-async-promise-executor rule in eslint

no, it's unrelated

see http://ember-concurrency.com/docs/tutorial (scroll down to Version 4)

⬆️

[rajasegar]

@Turbo87 Then, can you please help me in coming up with a proper brief about the rule

[rajasegar]

@bmish Regarding the error above, any help would be appreciated.

[Turbo87]

as written in the initial post:

see http://ember-concurrency.com/docs/tutorial (scroll down to Version 4)

tl;dr: using async actions can lead to memory leaks and application errors if you don't check for isDestroying and isDestroyed after each async step

[rajasegar]

Sounds good to a brief, but do we also need to mention how to fix this error or any recommended practice to avoid async actions.

[mongoose700]

I don't think this rule makes sense. There are going to be cases where we want to trigger asynchronous behavior through actions, but this doesn't address how we would do so. The good example is returning something other than a promise, but if we just had a promise that didn't get returned, this lint rule would just ignore it.

Given the stated goal is to avoid errors from not checking isDestroying or isDestroyed, it would make more sense for the lint rule to focus on that. If someone did correctly check those, it would still fail the lint rule as it's currently written. And if someone does handle it correctly, they shouldn't have to then manually disable the rule.

[Turbo87]

@mongoose700 this rule will be optional, if you don't want it, then you don't have to enable it.

There are going to be cases where we want to trigger asynchronous behavior through actions

you can trigger it, there is nothing wrong with that. it's just that doing anything with this after the async behavior has finished can be very dangerous.

[mongoose700]

@mongoose700 this rule will be optional, if you don't want it, then you don't have to enable it.

Even so, I think it should do a better job at targeting the behavior it's intended to discourage.

you can trigger it, there is nothing wrong with that. it's just that doing anything with this after the async behavior has finished can be very dangerous.

Then the rule should be focused on anything that may modify this asynchronously, not on just whether the function is async or returns a promise. If someone has an action that violates this rule, they are more likely to fix it by making it stop returning the promise than by adding the proper checks, especially since the error message is simply Do not use async actions. Do you believe that async actions should be avoided entirely? The rule would make the entire example in the tutorial invalid.

[Turbo87]

Do you believe that async actions should be avoided entirely?

yes, unless you know about isDestroying and isDestroyed and promise to always apply them properly.

or alternatively one can use ember-concurrency which avoids these issues completely...

I think it should do a better job at targeting the behavior it's intended to discourage

you are very welcome to work on improving this rule 😉