Skip to content

Commit 9042859

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2563 from NullVoxPopuli/nvp/template-lint-extract-rule-template-no-positional-data-test-selectors
Extract rule: template-no-positional-data-test-selectors
2 parents 53ca4e0 + 46d8a9d commit 9042859

4 files changed

Lines changed: 306 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-* params in curly invocations | | | |
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: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# ember/template-no-positional-data-test-selectors
2+
3+
<!-- end auto-generated rule header -->
4+
5+
## Motivation
6+
7+
[ember-test-selectors](https://github.com/simplabs/ember-test-selectors) is a very popular library that enables better element selectors for testing.
8+
9+
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).
10+
11+
That would look like:
12+
13+
```hbs
14+
{{some-thing data-test-foo}}
15+
```
16+
17+
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).
18+
19+
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)).
20+
21+
## Examples
22+
23+
This rule forbids the following:
24+
25+
```hbs
26+
{{foo-bar data-test-blah}}
27+
{{#foo-bar data-test-blah}}{{/foo-bar}}
28+
```
29+
30+
And suggests using the following instead:
31+
32+
```hbs
33+
{{foo-bar data-test-blah=true}}
34+
{{#foo-bar data-test-blah=true}}{{/foo-bar}}
35+
```
36+
37+
## References
38+
39+
- [ember-test-selectors#d47f73d](https://github.com/simplabs/ember-test-selectors/commit/d47f73d76b3ccbc9f0be5df3b897afd08b1636a6)

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
const BUILT_INS = new Set([
2+
'action',
3+
'array',
4+
'component',
5+
'concat',
6+
'debugger',
7+
'each',
8+
'each-in',
9+
'fn',
10+
'get',
11+
'hasBlock',
12+
'has-block',
13+
'has-block-params',
14+
'hash',
15+
'if',
16+
'input',
17+
'let',
18+
'link-to',
19+
'loc',
20+
'log',
21+
'mount',
22+
'mut',
23+
'on',
24+
'outlet',
25+
'partial',
26+
'query-params',
27+
'textarea',
28+
'unbound',
29+
'unless',
30+
'with',
31+
'-in-element',
32+
'in-element',
33+
]);
34+
35+
function checkNode(node, context) {
36+
if (!node.path || BUILT_INS.has(node.path.original)) {
37+
return;
38+
}
39+
40+
if (!node.params) {
41+
return;
42+
}
43+
44+
for (const param of node.params) {
45+
if (
46+
param.type === 'GlimmerPathExpression' &&
47+
param.original &&
48+
param.original.startsWith('data-test-')
49+
) {
50+
context.report({
51+
node,
52+
messageId: 'noPositionalDataTest',
53+
});
54+
return;
55+
}
56+
}
57+
}
58+
59+
/** @type {import('eslint').Rule.RuleModule} */
60+
module.exports = {
61+
meta: {
62+
type: 'suggestion',
63+
docs: {
64+
description: 'disallow positional data-test-* params in curly invocations',
65+
category: 'Best Practices',
66+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-positional-data-test-selectors.md',
67+
templateMode: 'both',
68+
},
69+
fixable: null,
70+
schema: [],
71+
messages: {
72+
noPositionalDataTest:
73+
'Passing a `data-test-*` positional param to a curly invocation should be avoided.',
74+
},
75+
originallyFrom: {
76+
name: 'ember-template-lint',
77+
rule: 'lib/rules/no-positional-data-test-selectors.js',
78+
docs: 'docs/rule/no-positional-data-test-selectors.md',
79+
tests: 'test/unit/rules/no-positional-data-test-selectors-test.js',
80+
},
81+
},
82+
83+
create(context) {
84+
return {
85+
GlimmerMustacheStatement(node) {
86+
checkNode(node, context);
87+
},
88+
89+
GlimmerBlockStatement(node) {
90+
checkNode(node, context);
91+
},
92+
};
93+
},
94+
};

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
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+
{{#if data-test-foo}}
21+
{{/if}}
22+
</template>`,
23+
`<template>
24+
<div data-test-blah></div>
25+
</template>`,
26+
`<template>
27+
<Foo data-test-derp />
28+
</template>`,
29+
`<template>
30+
{{something data-test-lol=true}}
31+
</template>`,
32+
`<template>
33+
{{#if dataSomething}}
34+
<div> hello </div>
35+
{{/if}}
36+
</template>`,
37+
`<template>
38+
<div
39+
data-test-msg-connections-typeahead-result={{true}}
40+
>
41+
</div>
42+
</template>`,
43+
`<template>
44+
<div
45+
data-test-msg-connections-typeahead-result="foo-bar"
46+
>
47+
</div>
48+
</template>`,
49+
`<template>
50+
{{badge
51+
data-test-profile-card-one-to-one-connection-distance=true
52+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
53+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
54+
}}
55+
</template>`,
56+
`<template>
57+
{{badge
58+
data-test-profile-card-one-to-one-connection-distance="foo-bar"
59+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
60+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
61+
}}
62+
</template>`,
63+
`<template>
64+
<div
65+
data-test-profile=true
66+
>
67+
hello
68+
</div>
69+
</template>`,
70+
],
71+
72+
invalid: [
73+
{
74+
code: `<template>
75+
{{badge
76+
data-test-profile-card-one-to-one-connection-distance
77+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
78+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
79+
}}
80+
</template>`,
81+
output: null,
82+
errors: [
83+
{
84+
message:
85+
'Passing a `data-test-*` positional param to a curly invocation should be avoided.',
86+
},
87+
],
88+
},
89+
],
90+
});
91+
92+
const hbsRuleTester = new RuleTester({
93+
parser: require.resolve('ember-eslint-parser/hbs'),
94+
parserOptions: {
95+
ecmaVersion: 2022,
96+
sourceType: 'module',
97+
},
98+
});
99+
100+
hbsRuleTester.run('template-no-positional-data-test-selectors', rule, {
101+
valid: [
102+
`
103+
{{#if data-test-foo}}
104+
{{/if}}
105+
`,
106+
`
107+
<div data-test-blah></div>
108+
`,
109+
`
110+
<Foo data-test-derp />
111+
`,
112+
`
113+
{{something data-test-lol=true}}
114+
`,
115+
`
116+
{{#if dataSomething}}
117+
<div> hello </div>
118+
{{/if}}
119+
`,
120+
`
121+
<div
122+
data-test-msg-connections-typeahead-result={{true}}
123+
>
124+
</div>
125+
`,
126+
`
127+
<div
128+
data-test-msg-connections-typeahead-result="foo-bar"
129+
>
130+
</div>
131+
`,
132+
`
133+
{{badge
134+
data-test-profile-card-one-to-one-connection-distance=true
135+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
136+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
137+
}}
138+
`,
139+
`
140+
{{badge
141+
data-test-profile-card-one-to-one-connection-distance="foo-bar"
142+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
143+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
144+
}}
145+
`,
146+
`
147+
<div
148+
data-test-profile=true
149+
>
150+
hello
151+
</div>
152+
`,
153+
],
154+
invalid: [
155+
{
156+
code: `
157+
{{badge
158+
data-test-profile-card-one-to-one-connection-distance
159+
degreeText=(t "i18n_distance_v2" distance=recipientDistance)
160+
degreeA11yText=(t "i18n_distance_a11y_v2" distance=recipientDistance)
161+
}}
162+
`,
163+
output: null,
164+
errors: [
165+
{
166+
message:
167+
'Passing a `data-test-*` positional param to a curly invocation should be avoided.',
168+
},
169+
],
170+
},
171+
],
172+
});

0 commit comments

Comments
 (0)