Skip to content

Commit 4998048

Browse files
Merge pull request #2562 from NullVoxPopuli/nvp/template-lint-extract-rule-template-no-passed-in-event-handlers
Extract rule: template-no-passed-in-event-handlers
2 parents 8952249 + afd1342 commit 4998048

4 files changed

Lines changed: 510 additions & 0 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ rules in templates can be disabled with eslint directives with mustache or html
259259
| [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | |
260260
| [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | |
261261
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
262+
| [template-no-passed-in-event-handlers](docs/rules/template-no-passed-in-event-handlers.md) | disallow passing event handlers directly as component arguments | | | |
262263
| [template-no-positional-data-test-selectors](docs/rules/template-no-positional-data-test-selectors.md) | disallow positional data-test-* params in curly invocations | | | |
263264
| [template-no-potential-path-strings](docs/rules/template-no-potential-path-strings.md) | disallow potential path strings in attribute values | | | |
264265
| [template-no-redundant-fn](docs/rules/template-no-redundant-fn.md) | disallow unnecessary usage of (fn) helper | | | |
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
# ember/template-no-passed-in-event-handlers
2+
3+
<!-- end auto-generated rule header -->
4+
5+
It is possible to pass e.g. `@click` to an Ember component to override the default `click` event handler. For tagless components this will trigger an assertion though and can't be used as legitimate API, and for Glimmer components it will not work out of the box, like in Ember components, either.
6+
7+
This rule scans potential component invocations for these patterns and flags them as issues.
8+
9+
## Examples
10+
11+
This rule **forbids** the following:
12+
13+
```hbs
14+
<Foo @click={{this.handleClick}} />
15+
```
16+
17+
```hbs
18+
<Foo @keyPress={{this.handleClick}} />
19+
```
20+
21+
```hbs
22+
{{foo click=this.handleClick}}
23+
```
24+
25+
This rule **allows** the following:
26+
27+
```hbs
28+
<Foo @onClick={{this.handleClick}} />
29+
```
30+
31+
```hbs
32+
<Foo @myCustomClickHandler={{this.handleClick}} />
33+
```
34+
35+
```hbs
36+
<Foo @onKeyPress={{this.handleClick}} />
37+
```
38+
39+
```hbs
40+
{{foo onClick=this.handleClick}}
41+
```
42+
43+
## Configuration
44+
45+
- boolean - `true` to enable / `false` to disable
46+
- object -- An object with the following keys:
47+
- `ignore` -- An object with the following keys/values:
48+
- key: string -- The name of the element or mustache statement to ignore event handlers for
49+
- value: array -- Event handler names. Event handler names should exclude the @ when specifying those intended for named arguments. This rule will ensure both non-named arguments and named arguments are both ignored appropriately.
50+
51+
eg.
52+
53+
Given the following configuration:
54+
55+
```json
56+
{
57+
"ignore": {
58+
"MyButton": ["click"]
59+
}
60+
}
61+
```
62+
63+
## Migration
64+
65+
- create explicit component APIs for these events (e.g. `@click` -> `@onClick`)
66+
67+
## References
68+
69+
- <https://api.emberjs.com/ember/release/classes/Component#event-handler-methods>
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Comprehensive Ember event handler names
2+
const EMBER_EVENTS = new Set([
3+
'touchStart',
4+
'touchMove',
5+
'touchEnd',
6+
'touchCancel',
7+
'keyDown',
8+
'keyUp',
9+
'keyPress',
10+
'mouseDown',
11+
'mouseUp',
12+
'contextMenu',
13+
'click',
14+
'doubleClick',
15+
'mouseMove',
16+
'mouseEnter',
17+
'mouseLeave',
18+
'focusIn',
19+
'focusOut',
20+
'submit',
21+
'change',
22+
'input',
23+
'dragStart',
24+
'drag',
25+
'dragEnter',
26+
'dragLeave',
27+
'dragOver',
28+
'dragEnd',
29+
'drop',
30+
]);
31+
32+
function isEventName(name) {
33+
return EMBER_EVENTS.has(name);
34+
}
35+
36+
/** @type {import('eslint').Rule.RuleModule} */
37+
module.exports = {
38+
meta: {
39+
type: 'suggestion',
40+
docs: {
41+
description: 'disallow passing event handlers directly as component arguments',
42+
category: 'Best Practices',
43+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-passed-in-event-handlers.md',
44+
templateMode: 'both',
45+
},
46+
fixable: null,
47+
schema: [
48+
{
49+
type: 'object',
50+
properties: {
51+
ignore: {
52+
type: 'object',
53+
additionalProperties: {
54+
type: 'array',
55+
items: { type: 'string' },
56+
},
57+
},
58+
},
59+
additionalProperties: false,
60+
},
61+
],
62+
messages: {
63+
unexpected:
64+
'Event handler "@{{name}}" should not be passed as a component argument. Use the `on` modifier instead.',
65+
},
66+
originallyFrom: {
67+
name: 'ember-template-lint',
68+
rule: 'lib/rules/no-passed-in-event-handlers.js',
69+
docs: 'docs/rule/no-passed-in-event-handlers.md',
70+
tests: 'test/unit/rules/no-passed-in-event-handlers-test.js',
71+
},
72+
},
73+
74+
create(context) {
75+
const options = context.options[0] || {};
76+
const ignoreConfig = options.ignore || {};
77+
78+
return {
79+
GlimmerElementNode(node) {
80+
// Only check component invocations (PascalCase)
81+
if (!/^[A-Z]/.test(node.tag)) {
82+
return;
83+
}
84+
// Skip built-in Input/Textarea
85+
if (node.tag === 'Input' || node.tag === 'Textarea') {
86+
return;
87+
}
88+
89+
if (!node.attributes) {
90+
return;
91+
}
92+
93+
const ignoredAttrs = ignoreConfig[node.tag] || [];
94+
95+
for (const attr of node.attributes) {
96+
if (!attr.name || !attr.name.startsWith('@')) {
97+
continue;
98+
}
99+
const argName = attr.name.slice(1);
100+
101+
// Check ignore config
102+
if (ignoredAttrs.includes(attr.name)) {
103+
continue;
104+
}
105+
106+
if (isEventName(argName)) {
107+
context.report({
108+
node: attr,
109+
messageId: 'unexpected',
110+
data: { name: argName },
111+
});
112+
}
113+
}
114+
},
115+
116+
GlimmerMustacheStatement(node) {
117+
const path = node.path;
118+
if (!path || path.type !== 'GlimmerPathExpression') {
119+
return;
120+
}
121+
// Skip built-in input/textarea
122+
if (path.original === 'input' || path.original === 'textarea') {
123+
return;
124+
}
125+
// Check hash pairs for event handler names
126+
if (!node.hash || !node.hash.pairs) {
127+
return;
128+
}
129+
const ignoredAttrs = ignoreConfig[path.original] || [];
130+
131+
for (const pair of node.hash.pairs) {
132+
if (ignoredAttrs.includes(pair.key)) {
133+
continue;
134+
}
135+
if (isEventName(pair.key)) {
136+
context.report({
137+
node: pair,
138+
messageId: 'unexpected',
139+
data: { name: pair.key },
140+
});
141+
}
142+
}
143+
},
144+
};
145+
},
146+
};

0 commit comments

Comments
 (0)