Skip to content

Commit 85ac3df

Browse files
NullVoxPopuliNullVoxPopuli
committed
Extract rule: template-no-positional-data-test-selectors
1 parent 53ca4e0 commit 85ac3df

4 files changed

Lines changed: 357 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ rules in templates can be disabled with eslint directives with mustache or html
258258
| [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | |
259259
| [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | |
260260
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
261+
| [template-no-positional-data-test-selectors](docs/rules/template-no-positional-data-test-selectors.md) | disallow positional data-test selectors | | | |
261262
| [template-no-potential-path-strings](docs/rules/template-no-potential-path-strings.md) | disallow potential path strings in attribute values | | | |
262263
| [template-no-splattributes-with-class](docs/rules/template-no-splattributes-with-class.md) | disallow splattributes with class attribute | | | |
263264
| [template-no-trailing-spaces](docs/rules/template-no-trailing-spaces.md) | disallow trailing whitespace at the end of lines in templates | | 🔧 | |

‎docs/rules/template-no-positional-data-test-selectors.md‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# ember/template-no-positional-data-test-selectors
2+
3+
<!-- end auto-generated rule header -->
4+
5+
Disallows positional data-test selectors.
6+
7+
## Rule Details
8+
9+
Data-test selectors should use descriptive names rather than positional indices for better maintainability and clarity.
10+
11+
## Motivation
12+
13+
[ember-test-selectors](https://github.com/simplabs/ember-test-selectors) is a very popular library that enables better element selectors for testing.
14+
15+
One of the features that had been added to ember-test-selectors over the years was to allow passing a positional argument to curly component invocations as a shorthand (to avoid having to also add a named argument value).
16+
17+
That would look like:
18+
19+
```hbs
20+
{{some-thing data-test-foo}}
21+
```
22+
23+
Internally, that was converted to an `attributeBinding` for `@ember/component`s. Unfortunately, that particular invocation syntax is in conflict with modern Ember Octane templates. For example, in the snippet above `data-test-foo` is actually referring to `this.data-test-foo` (and would be marked as an error by the `no-implicit-this` rule).
24+
25+
Additionally, the nature of these "fake" local properties significantly confuses the codemods that are used to transition an application into Ember Octane (e.g. [ember-no-implicit-this-codemod](https://github.com/ember-codemods/ember-no-implicit-this-codemod) and [ember-angle-brackets-codemod](https://github.com/ember-codemods/ember-angle-brackets-codemod)).
26+
27+
## Examples
28+
29+
This rule checks two things:
30+
31+
1. **Curly component invocations** with positional `data-test-*` params (e.g. `{{badge data-test-profile-card}}`), which should use named arguments instead (e.g. `data-test-profile-card=true`).
32+
2. **HTML attribute values** that are purely numeric (e.g. `data-test-item="0"`), which should use descriptive names instead.
33+
34+
Examples of **incorrect** code for this rule:
35+
36+
```gjs
37+
<template>
38+
{{badge data-test-profile-card}}
39+
</template>
40+
```
41+
42+
```gjs
43+
<template>
44+
<div data-test-item="0"></div>
45+
</template>
46+
```
47+
48+
```gjs
49+
<template>
50+
<div data-test-card="1"></div>
51+
</template>
52+
```
53+
54+
Examples of **correct** code for this rule:
55+
56+
```gjs
57+
<template>
58+
{{badge data-test-profile-card=true}}
59+
</template>
60+
```
61+
62+
```gjs
63+
<template>
64+
<div data-test-user-card></div>
65+
</template>
66+
```
67+
68+
```gjs
69+
<template>
70+
<div data-test-item="my-item"></div>
71+
</template>
72+
```
73+
74+
## References
75+
76+
- [eslint-plugin-ember template-no-positional-data-test-selectors](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/template-no-positional-data-test-selectors.md)

‎lib/rules/template-no-positional-data-test-selectors.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/** @type {import('eslint').Rule.RuleModule} */
2+
module.exports = {
3+
meta: {
4+
type: 'suggestion',
5+
docs: {
6+
description: 'disallow positional data-test selectors',
7+
category: 'Best Practices',
8+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-positional-data-test-selectors.md',
9+
templateMode: 'both',
10+
},
11+
fixable: null,
12+
schema: [],
13+
messages: {
14+
noPositionalDataTest:
15+
'Use named data-test attributes instead of positional data-test-* attributes.',
16+
},
17+
originallyFrom: {
18+
name: 'ember-template-lint',
19+
rule: 'lib/rules/no-positional-data-test-selectors.js',
20+
docs: 'docs/rule/no-positional-data-test-selectors.md',
21+
tests: 'test/unit/rules/no-positional-data-test-selectors-test.js',
22+
},
23+
},
24+
25+
create(context) {
26+
return {
27+
GlimmerElementNode(node) {
28+
if (!node.attributes) {
29+
return;
30+
}
31+
32+
for (const attr of node.attributes) {
33+
if (attr.type === 'GlimmerAttrNode' && attr.name && attr.name.startsWith('data-test-')) {
34+
// Check if it's a positional selector (has a value that looks like an index)
35+
if (attr.value && attr.value.type === 'GlimmerTextNode') {
36+
const value = attr.value.chars;
37+
if (/^\d+$/.test(value)) {
38+
context.report({
39+
node: attr,
40+
messageId: 'noPositionalDataTest',
41+
});
42+
}
43+
}
44+
}
45+
}
46+
},
47+
48+
GlimmerMustacheStatement(node) {
49+
if (!node.params) {
50+
return;
51+
}
52+
for (const param of node.params) {
53+
if (
54+
param.type === 'GlimmerPathExpression' &&
55+
param.original &&
56+
param.original.startsWith('data-test-')
57+
) {
58+
context.report({
59+
node: param,
60+
messageId: 'noPositionalDataTest',
61+
});
62+
}
63+
}
64+
},
65+
};
66+
},
67+
};

‎tests/lib/rules/template-no-positional-data-test-selectors.js‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
//------------------------------------------------------------------------------
2+
// Requirements
3+
//------------------------------------------------------------------------------
4+
5+
const rule = require('../../../lib/rules/template-no-positional-data-test-selectors');
6+
const RuleTester = require('eslint').RuleTester;
7+
8+
//------------------------------------------------------------------------------
9+
// Tests
10+
//------------------------------------------------------------------------------
11+
12+
const ruleTester = new RuleTester({
13+
parser: require.resolve('ember-eslint-parser'),
14+
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
15+
});
16+
17+
ruleTester.run('template-no-positional-data-test-selectors', rule, {
18+
valid: [
19+
`<template>
20+
<div data-test-user-card></div>
21+
</template>`,
22+
`<template>
23+
<div data-test-item="my-item"></div>
24+
</template>`,
25+
`<template>
26+
<button data-test-button></button>
27+
</template>`,
28+
29+
`<template>
30+
{{#if data-test-foo}}
31+
{{/if}}
32+
</template>`,
33+
`<template>
34+
<div data-test-blah></div>
35+
</template>`,
36+
`<template>
37+
<Foo data-test-derp />
38+
</template>`,
39+
`<template>
40+
{{something data-test-lol=true}}
41+
</template>`,
42+
`<template>
43+
{{#if dataSomething}}
44+
<div> hello </div>
45+
{{/if}}
46+
</template>`,
47+
`<template>
48+
<div
49+
data-test-msg-connections-typeahead-result={{true}}
50+
>
51+
</div>
52+
</template>`,
53+
`<template>
54+
<div
55+
data-test-msg-connections-typeahead-result="foo-bar"
56+
>
57+
</div>
58+
</template>`,
59+
`<template>
60+
{{badge
61+
data-test-profile-card-one-to-one-connection-distance=true
62+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
63+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
64+
}}
65+
</template>`,
66+
`<template>
67+
{{badge
68+
data-test-profile-card-one-to-one-connection-distance="foo-bar"
69+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
70+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
71+
}}
72+
</template>`,
73+
`<template>
74+
<div
75+
data-test-profile=true
76+
>
77+
hello
78+
</div>
79+
</template>`,
80+
],
81+
82+
invalid: [
83+
{
84+
code: `<template>
85+
<div data-test-item="0"></div>
86+
</template>`,
87+
output: null,
88+
errors: [
89+
{
90+
message: 'Use named data-test attributes instead of positional data-test-* attributes.',
91+
type: 'GlimmerAttrNode',
92+
},
93+
],
94+
},
95+
{
96+
code: `<template>
97+
<div data-test-card="1"></div>
98+
</template>`,
99+
output: null,
100+
errors: [
101+
{
102+
message: 'Use named data-test attributes instead of positional data-test-* attributes.',
103+
type: 'GlimmerAttrNode',
104+
},
105+
],
106+
},
107+
{
108+
code: `<template>
109+
<button data-test-button="123"></button>
110+
</template>`,
111+
output: null,
112+
errors: [
113+
{
114+
message: 'Use named data-test attributes instead of positional data-test-* attributes.',
115+
type: 'GlimmerAttrNode',
116+
},
117+
],
118+
},
119+
120+
{
121+
code: `<template>
122+
{{badge
123+
data-test-profile-card-one-to-one-connection-distance
124+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
125+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
126+
}}
127+
</template>`,
128+
output: null,
129+
errors: [
130+
{ message: 'Use named data-test attributes instead of positional data-test-* attributes.' },
131+
],
132+
},
133+
],
134+
});
135+
136+
const hbsRuleTester = new RuleTester({
137+
parser: require.resolve('ember-eslint-parser/hbs'),
138+
parserOptions: {
139+
ecmaVersion: 2022,
140+
sourceType: 'module',
141+
},
142+
});
143+
144+
hbsRuleTester.run('template-no-positional-data-test-selectors', rule, {
145+
valid: [
146+
`
147+
{{#if data-test-foo}}
148+
{{/if}}
149+
`,
150+
`
151+
<div data-test-blah></div>
152+
`,
153+
`
154+
<Foo data-test-derp />
155+
`,
156+
`
157+
{{something data-test-lol=true}}
158+
`,
159+
`
160+
{{#if dataSomething}}
161+
<div> hello </div>
162+
{{/if}}
163+
`,
164+
`
165+
<div
166+
data-test-msg-connections-typeahead-result={{true}}
167+
>
168+
</div>
169+
`,
170+
`
171+
<div
172+
data-test-msg-connections-typeahead-result="foo-bar"
173+
>
174+
</div>
175+
`,
176+
`
177+
{{badge
178+
data-test-profile-card-one-to-one-connection-distance=true
179+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
180+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
181+
}}
182+
`,
183+
`
184+
{{badge
185+
data-test-profile-card-one-to-one-connection-distance="foo-bar"
186+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
187+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
188+
}}
189+
`,
190+
`
191+
<div
192+
data-test-profile=true
193+
>
194+
hello
195+
</div>
196+
`,
197+
],
198+
invalid: [
199+
{
200+
code: `
201+
{{badge
202+
data-test-profile-card-one-to-one-connection-distance
203+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
204+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
205+
}}
206+
`,
207+
output: null,
208+
errors: [
209+
{ message: 'Use named data-test attributes instead of positional data-test-* attributes.' },
210+
],
211+
},
212+
],
213+
});

0 commit comments

Comments
 (0)