Skip to content

Commit 97c7d56

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2612 from NullVoxPopuli/nvp/template-lint-extract-rule-template-require-splattributes
Extract rule: template-require-splattributes
2 parents e575c19 + b1d64a8 commit 97c7d56

4 files changed

Lines changed: 254 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ rules in templates can be disabled with eslint directives with mustache or html
247247
| [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | |
248248
| [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | |
249249
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
250+
| [template-require-splattributes](docs/rules/template-require-splattributes.md) | require splattributes usage in component templates | | | |
250251
| [template-require-strict-mode](docs/rules/template-require-strict-mode.md) | require templates to be in strict mode | | | |
251252
| [template-require-valid-named-block-naming-format](docs/rules/template-require-valid-named-block-naming-format.md) | require valid named block naming format | | 🔧 | |
252253
| [template-self-closing-void-elements](docs/rules/template-self-closing-void-elements.md) | require self-closing on void elements | | 🔧 | |

‎docs/rules/template-require-splattributes.md‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# ember/template-require-splattributes
2+
3+
<!-- end auto-generated rule header -->
4+
5+
Require splattributes usage in component templates.
6+
7+
Splattributes (`...attributes`) make it possible to use attributes on component
8+
invocations (e.g. `<SomeComponent class="blue">`). Forgetting to add
9+
`...attributes` however makes it impossible to apply attributes like `class` to
10+
a component.
11+
12+
This rule warns about templates that don't have `...attributes` in them.
13+
14+
Please note that this rule is only useful for Glimmer components or tagless
15+
(`tagName: ''`) classic components, because regular classic components have
16+
this functionality built into the root element, which is not part of their
17+
templates.
18+
19+
This rule also should not be used for route/controller templates, because those
20+
don't support `...attributes`. Instead of unconditionally enabling this rule in
21+
your config, you might want to consider using overrides to only enable it for
22+
component templates.
23+
24+
## Examples
25+
26+
This rule **forbids** the following:
27+
28+
```gjs
29+
<template>
30+
<div>
31+
component content
32+
</div>
33+
</template>
34+
```
35+
36+
```gjs
37+
<template>
38+
<SomeOtherComponent>
39+
component content
40+
</SomeOtherComponent>
41+
</template>
42+
```
43+
44+
This rule **allows** the following:
45+
46+
```gjs
47+
<template>
48+
<div ...attributes>
49+
component content
50+
</div>
51+
</template>
52+
```
53+
54+
```gjs
55+
<template>
56+
<div class="foo">
57+
<SomeOtherComponent ...attributes />
58+
</div>
59+
</template>
60+
```
61+
62+
```js
63+
module.exports = {
64+
extends: 'recommended',
65+
rules: {
66+
// ...
67+
},
68+
overrides: [
69+
{
70+
files: ['app/components/**/*.hbs'],
71+
rules: { 'require-splattributes': 'error' },
72+
},
73+
],
74+
};
75+
```
76+
77+
## Migration
78+
79+
- Add `...attributes` on at least one element or component invocation in the template (usually the root element)
80+
- Use `{{! template-lint-disable require-splattributes }}` where you explicitly don't want or need `...attributes`
81+
82+
## Related Rules
83+
84+
- [no-nested-splattributes](template-no-nested-splattributes.md)
85+
- [splat-attributes-only](template-splat-attributes-only.md)
86+
87+
## References
88+
89+
- [Splattributes](https://guides.emberjs.com/release/components/component-arguments-and-html-attributes/#toc_html-attributes) in the Ember.js guides

‎lib/rules/template-require-splattributes.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/** @type {import('eslint').Rule.RuleModule} */
2+
module.exports = {
3+
meta: {
4+
type: 'suggestion',
5+
docs: {
6+
description: 'require splattributes usage in component templates',
7+
category: 'Best Practices',
8+
recommended: false,
9+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-require-splattributes.md',
10+
templateMode: 'both',
11+
},
12+
fixable: null,
13+
schema: [],
14+
messages: {
15+
rootElement: 'The root element in this template should use `...attributes`',
16+
atLeastOne: 'At least one element in this template should use `...attributes`',
17+
},
18+
originallyFrom: {
19+
name: 'ember-template-lint',
20+
rule: 'lib/rules/require-splattributes.js',
21+
docs: 'docs/rule/require-splattributes.md',
22+
tests: 'test/unit/rules/require-splattributes-test.js',
23+
},
24+
},
25+
26+
create(context) {
27+
let foundSplattributes = false;
28+
29+
return {
30+
GlimmerAttrNode(node) {
31+
if (node.name === '...attributes') {
32+
foundSplattributes = true;
33+
}
34+
},
35+
36+
'GlimmerTemplate:exit'(node) {
37+
if (foundSplattributes) {
38+
return;
39+
}
40+
41+
const body = node.body ?? [];
42+
const effectiveBody =
43+
body.length === 1 &&
44+
body[0].type === 'GlimmerElementNode' &&
45+
body[0].tag === 'template' &&
46+
Array.isArray(body[0].children)
47+
? body[0].children
48+
: body;
49+
const elementNodes = effectiveBody.filter((child) => child.type === 'GlimmerElementNode');
50+
const significantTextNodes = effectiveBody.filter(
51+
(child) => child.type === 'GlimmerTextNode' && child.chars.trim() !== ''
52+
);
53+
const hasOnlyOneElement = elementNodes.length === 1 && significantTextNodes.length === 0;
54+
55+
if (hasOnlyOneElement) {
56+
context.report({
57+
node: elementNodes[0],
58+
messageId: 'rootElement',
59+
});
60+
} else {
61+
context.report({
62+
node,
63+
messageId: 'atLeastOne',
64+
});
65+
}
66+
},
67+
};
68+
},
69+
};

‎tests/lib/rules/template-require-splattributes.js‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
const rule = require('../../../lib/rules/template-require-splattributes');
2+
const RuleTester = require('eslint').RuleTester;
3+
4+
const ruleTester = new RuleTester({
5+
parser: require.resolve('ember-eslint-parser'),
6+
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
7+
});
8+
9+
ruleTester.run('template-require-splattributes', rule, {
10+
valid: [
11+
'<template><div ...attributes></div></template>',
12+
'<template><Foo ...attributes></Foo></template>',
13+
'<template><div ...attributes /></template>',
14+
'<template><div><Foo ...attributes /></div></template>',
15+
'<template><div ...attributes></div><div></div></template>',
16+
17+
'<template><div></div><div ...attributes></div><div></div></template>',
18+
],
19+
invalid: [
20+
{
21+
code: '<template><div></div></template>',
22+
output: null,
23+
errors: [
24+
{
25+
message: 'The root element in this template should use `...attributes`',
26+
},
27+
],
28+
},
29+
{
30+
code: '<template><Foo></Foo></template>',
31+
output: null,
32+
errors: [
33+
{
34+
message: 'The root element in this template should use `...attributes`',
35+
},
36+
],
37+
},
38+
39+
{
40+
code: '<template><div></div><div></div></template>',
41+
output: null,
42+
errors: [{ message: 'At least one element in this template should use `...attributes`' }],
43+
},
44+
{
45+
code: `<template><div/>
46+
47+
</template>`,
48+
output: null,
49+
errors: [{ message: 'The root element in this template should use `...attributes`' }],
50+
},
51+
],
52+
});
53+
54+
const hbsRuleTester = new RuleTester({
55+
parser: require.resolve('ember-eslint-parser/hbs'),
56+
parserOptions: {
57+
ecmaVersion: 2022,
58+
sourceType: 'module',
59+
},
60+
});
61+
62+
hbsRuleTester.run('template-require-splattributes', rule, {
63+
valid: [
64+
'<div ...attributes></div>',
65+
'<Foo ...attributes></Foo>',
66+
'<div ...attributes />',
67+
'<div><Foo ...attributes /></div>',
68+
'<div ...attributes></div><div></div>',
69+
'<div></div><div ...attributes></div><div></div>',
70+
],
71+
invalid: [
72+
{
73+
code: '<div></div>',
74+
output: null,
75+
errors: [{ message: 'The root element in this template should use `...attributes`' }],
76+
},
77+
{
78+
code: '<Foo></Foo>',
79+
output: null,
80+
errors: [{ message: 'The root element in this template should use `...attributes`' }],
81+
},
82+
{
83+
code: '<div></div><div></div>',
84+
output: null,
85+
errors: [{ message: 'At least one element in this template should use `...attributes`' }],
86+
},
87+
{
88+
code: `<div/>
89+
90+
`,
91+
output: null,
92+
errors: [{ message: 'The root element in this template should use `...attributes`' }],
93+
},
94+
],
95+
});

0 commit comments

Comments
 (0)