Skip to content

Commit b5f341c

Browse files
committed
feat: add new rule 'no-element-event-actions'.
1 parent 0289efb commit b5f341c

File tree

6 files changed

+127
-0
lines changed

6 files changed

+127
-0
lines changed

docs/rule/no-element-event-actions.md

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
## no-element-event-actions
2+
3+
Using HTML element event properties such as `onclick` for Ember actions is not recommended for the following reasons:
4+
5+
* It doesn't work for SVGs (since there is no `onclick` property of `SVGElement`).
6+
* It can lead to confusing and unexpected behavior when mixed with normal action usage. For a comprehensive explanation of why, read [Deep Dive on Ember Events].
7+
8+
This rule **forbids** the following:
9+
10+
```hbs
11+
<button onclick={{action "submit"}}>Submit</button>
12+
```
13+
14+
This rule **allows** the following:
15+
16+
```hbs
17+
<button {{action "submit"}}>Submit</button>
18+
```
19+
20+
```hbs
21+
<button {{action "submit" on="doubleClick"}}>Submit On Double Click</button>
22+
```
23+
24+
### References
25+
26+
* [Deep Dive on Ember Events]
27+
* Ember [Template Action](https://guides.emberjs.com/release/templates/actions/) documentation
28+
* [List of DOM Events](https://developer.mozilla.org/en-US/docs/Web/Events)
29+
30+
[Deep Dive on Ember Events]: https://medium.com/square-corner-blog/deep-dive-on-ember-events-cf684fd3b808

docs/rules.md

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* [no-bare-strings](rule/no-bare-strings.md)
1515
* [no-debugger](rule/no-debugger.md)
1616
* [no-duplicate-attributes](rule/no-duplicate-attributes.md)
17+
* [no-element-event-actions](rule/no-element-event-actions.md)
1718
* [no-extra-mut-helper-argument](rule/no-extra-mut-helper-argument.md)
1819
* [no-html-comments](rule/no-html-comments.md)
1920
* [no-implicit-this](rule/no-implicit-this.md)

lib/helpers/ast-node-info.js

+5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ function isTextNode(node) {
1212
return node.type === 'TextNode';
1313
}
1414

15+
function isAttrNode(node) {
16+
return node.type === 'AttrNode';
17+
}
18+
1519
function isCommentStatement(node) {
1620
return node.type === 'CommentStatement';
1721
}
@@ -121,4 +125,5 @@ module.exports = {
121125
isMustacheStatement: isMustacheStatement,
122126
isNonConfigurationHtmlComment: isNonConfigurationHtmlComment,
123127
isTextNode: isTextNode,
128+
isAttrNode: isAttrNode,
124129
};

lib/rules/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ module.exports = {
1818
'no-bare-strings': require('./lint-no-bare-strings'),
1919
'no-debugger': require('./lint-no-debugger'),
2020
'no-duplicate-attributes': require('./lint-duplicate-attributes'),
21+
'no-element-event-actions': require('./lint-no-element-event-actions'),
2122
'no-extra-mut-helper-argument': require('./lint-no-extra-mut-helper-argument'),
2223
'no-html-comments': require('./lint-no-html-comments'),
2324
'no-implicit-this': require('./lint-no-implicit-this'),
+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
3+
const Rule = require('./base');
4+
const AstNodeInfo = require('../helpers/ast-node-info');
5+
6+
const ERROR_MESSAGE = 'Do not use HTML element event properties with Ember actions.';
7+
8+
module.exports = class NoElementEventActions extends Rule {
9+
visitor() {
10+
return {
11+
ElementNode(node) {
12+
const eventProperties = node.attributes.filter(isEventPropertyWithAction);
13+
eventProperties.forEach(eventProperty =>
14+
this.log({
15+
message: ERROR_MESSAGE,
16+
line: eventProperty.loc && eventProperty.loc.start.line,
17+
column: eventProperty.loc && eventProperty.loc.start.column,
18+
source: this.sourceForNode(eventProperty),
19+
})
20+
);
21+
},
22+
};
23+
}
24+
};
25+
26+
function isEventPropertyWithAction(node) {
27+
return (
28+
AstNodeInfo.isAttrNode(node) &&
29+
node.name.startsWith('on') &&
30+
node.name !== 'on' &&
31+
AstNodeInfo.isMustacheStatement(node.value) &&
32+
AstNodeInfo.isPathExpression(node.value.path) &&
33+
node.value.path.original === 'action'
34+
);
35+
}
36+
37+
module.exports.ERROR_MESSAGE = ERROR_MESSAGE;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
const generateRuleTests = require('../../helpers/rule-test-harness');
4+
const ERROR_MESSAGE = require('../../../lib/rules/lint-no-element-event-actions').ERROR_MESSAGE;
5+
6+
generateRuleTests({
7+
name: 'no-element-event-actions',
8+
9+
config: true,
10+
11+
good: [
12+
'<button></button>',
13+
'<button type="button" on={{action "myAction"}}></button>',
14+
'<button type="button" onclick="myFunction()"></button>',
15+
'<button type="button" {{action "myAction"}}></button>',
16+
'<button type="button" value={{value}}></button>',
17+
'{{my-component onclick=(action "myAction") someProperty=true}}',
18+
'<SiteHeader @someFunction={{action "myAction"}} @user={{this.user}} />',
19+
],
20+
21+
bad: [
22+
{
23+
template:
24+
'<button onclick={{action "myAction"}} onfocus={{action "myAction"}} otherProperty=true></button>',
25+
26+
results: [
27+
{
28+
message: ERROR_MESSAGE,
29+
source: 'onclick={{action "myAction"}}',
30+
line: 1,
31+
column: 8,
32+
},
33+
{
34+
message: ERROR_MESSAGE,
35+
source: 'onfocus={{action "myAction"}}',
36+
line: 1,
37+
column: 38,
38+
},
39+
],
40+
},
41+
42+
{
43+
template: '<SiteHeader onclick={{action "myAction"}} @user={{this.user}} />',
44+
45+
result: {
46+
message: ERROR_MESSAGE,
47+
source: 'onclick={{action "myAction"}}',
48+
line: 1,
49+
column: 12,
50+
},
51+
},
52+
],
53+
});

0 commit comments

Comments
 (0)