Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ The `--fix` option on the command line automatically fixes problems reported by
| | Rule ID | Description |
|:---|:--------|:------------|
| :white_check_mark: | [jquery-ember-run](./docs/rules/jquery-ember-run.md) | Prevents usage of jQuery without Ember Run Loop |
| | [no-async-actions](./docs/rules/no-async-actions.md) | Disallow usage of async actions in components |
| :white_check_mark: | [no-attrs-in-components](./docs/rules/no-attrs-in-components.md) | Disallow usage of this.attrs in components |
| :white_check_mark: | [no-attrs-snapshot](./docs/rules/no-attrs-snapshot.md) | Disallow use of attrs snapshot in `didReceiveAttrs` and `didUpdateAttrs` |
| :white_check_mark: | [no-capital-letters-in-routes](./docs/rules/no-capital-letters-in-routes.md) | Raise an error when there is a route with uppercased letters in router.js |
Expand Down
52 changes: 52 additions & 0 deletions docs/rules/no-async-actions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Do not use async actions
## Rule `no-async-actions`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, could we throw a lint error only after an await AND access of this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, could we throw a lint error only after an await AND access of this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example?: #1421



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit unclear on why we should not use async actions. Can you explain that more and include in the doc here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too not very clear about the requirement, i am exploring , will add more info once ready

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add at least a brief explanation of the reasoning in this doc.


Examples of **incorrect** code for this rule:
```js
actions: {
async handleClick() {
// ...
}
}
```

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

```js
@action
async handleClick() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why forbid this? This is totally valid

// ...
}
```

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


Examples of **correct** code for this rule:
```js
actions: {
handleClick() {
return nothingOrSomethingThatIsNotAPromise;
}
}
```


## Further Reading

- Ember Concurrency http://ember-concurrency.com/docs/tutorial (scroll down to Version 4)
- eslint/no-async-promise-executor https://github.com/eslint/eslint/blob/master/docs/rules/no-async-promise-executor.md

3 changes: 2 additions & 1 deletion lib/recommended-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module.exports = {
"ember/local-modules": "off",
"ember/named-functions-in-promises": "off",
"ember/new-module-imports": "error",
"ember/no-async-actions": "off",
"ember/no-attrs-in-components": "error",
"ember/no-attrs-snapshot": "error",
"ember/no-capital-letters-in-routes": "error",
Expand Down Expand Up @@ -50,4 +51,4 @@ module.exports = {
"ember/routes-segments-snake-case": "error",
"ember/use-brace-expansion": "error",
"ember/use-ember-get-and-set": "off"
}
}
60 changes: 60 additions & 0 deletions lib/rules/no-async-actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict';

const utils = require('../utils/utils');


//------------------------------------------------------------------------------
// General rule - Don't use async actions
//------------------------------------------------------------------------------


const message = 'Do not use async actions';

module.exports = {
meta: {
docs: {
description: 'Disallow usage of async actions in components',
category: 'Possible Errors',
url: 'http://ember-concurrency.com/docs/tutorial'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase on the latest master to pickup the prettier changes.

},
fixable: null,
},

create(context) {
return {
Property(node) {
if (node.key.name === 'actions') {
const props = node.value.properties;

props.forEach((p) => {
const body = p.value.body.body;
if (p.value.async) {
context.report({
node: p,
message,
});
} else if (body.length === 1 && utils.isReturnStatement(body[0])) {
const retSt = body[0];
if (retSt.argument.type === 'CallExpression' &&
retSt.argument.callee.property.name === 'then') {
context.report({
node: retSt,
message,
});
}
}
});
} else if (node.decorators) {
if (node.decorators.find(d => d.expression.name === 'action')) {
if (node.value.async) {
context.report({
node,
message: 'Do not use async actions'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the message variable defined at the top of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

});
}
}
}
}
};
}
};
96 changes: 96 additions & 0 deletions tests/lib/rules/no-async-actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
'use strict';

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-async-actions.js');
const RuleTester = require('eslint').RuleTester;


//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const message = 'Do not use async actions';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message should be imported from the rule file. See the no-test-and-then rule for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
}
});

ruleTester.run('no-async-actions', rule, {
valid: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a valid example to ensure this rule does not trigger on actions in an irrelevant object/class. It should only trigger on the actions key inside an Ember controller/component/route.

Copy link
Member

@bmish bmish Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ still waiting on this rule to be limited to actions inside components only.

{
code: `
Component.extend({
actions: {
handleClick() {
// ...
}
}
});`,
}

],

invalid: [
{
code: `Component.extend({
actions: {
async handleClick() {
// ...
}
}
});`,
output: null,
errors: [{
message,
}]
},
{
code: `Component.extend({
actions: {
handleClick() {
return something.then(() => {
let hello = "world";
});
}
}
});`,
output: null,
errors: [{
message,
}]
},
{
code: `Component.extend({
@action
async handleClick() {
// ...
}
});`,
output: null,
errors: [{
message,
}]
},
{
code: `Component.extend({
@action
handleClick() {
return something.then(() => {
let hello = "world";
});
}
});`,
output: null,
errors: [{
message,
}]
},

]
});