Skip to content

Commit

Permalink
Account for only having template tag in `no-empty-glimmer-component-c…
Browse files Browse the repository at this point in the history
…lasses` rule (#1866)
  • Loading branch information
chrisrng committed May 19, 2023
1 parent a8efed7 commit 55b987c
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 2 deletions.
25 changes: 24 additions & 1 deletion docs/rules/no-empty-glimmer-component-classes.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ This rule will catch and prevent the use of empty backing classes for Glimmer co

## Rule Details

This rule aims to disallow the use of empty backing classes for Glimmer components when possible. Template-only Glimmer components where there is no backing class are much faster and lighter-weight than Glimmer components with backing classes, which are much lighter-weight than Ember components. Therefore, you should only have a backing class for a Glimmer component when absolutely necessary.
This rule aims to disallow the use of empty backing classes for Glimmer components when possible including only using ember template tags in your Glimmer component. Template-only Glimmer components where there is no backing class are much faster and lighter-weight than Glimmer components with backing classes, which are much lighter-weight than Ember components. Therefore, you should only have a backing class for a Glimmer component when absolutely necessary.

To fix violations of this rule:

Expand All @@ -26,6 +26,15 @@ import Component from '@glimmer/component';
class MyComponent extends Component {}
```

<!-- markdownlint-disable-next-line MD040 -->
```
import Component from '@glimmer/component';
export default class MyComponent extends Component {
<template>Hello World!</template>
}
```

Examples of **correct** code for this rule:

```js
Expand Down Expand Up @@ -54,7 +63,21 @@ import MyDecorator from 'my-decorator';
class MyComponent extends Component {}
```

<!-- markdownlint-disable-next-line MD040 -->
```
import Component from '@glimmer/component';
export default class MyComponent extends Component {
foo() {
return this.args.bar + this.args.baz;
}
<template>Hello World!</template>
}
```

## References

- [Glimmer Components - Octane Upgrade Guide](https://guides.emberjs.com/release/upgrading/current-edition/glimmer-components/)
- [Glimmer Components RFC](https://emberjs.github.io/rfcs/0416-glimmer-components.html)
- [First-Class Component Templates RFC](https://rfcs.emberjs.com/id/0779-first-class-component-templates/)
14 changes: 13 additions & 1 deletion lib/rules/no-empty-glimmer-component-classes.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
'use strict';

const { isGlimmerComponent } = require('../utils/ember');
const { isClassPropertyOrPropertyDefinition } = require('../utils/types');

const ERROR_MESSAGE = 'Do not create empty backing classes for Glimmer components.';
const ERROR_MESSAGE_TEMPLATE_TAG =
'Do not create empty backing classes for Glimmer template tag only components.';

//------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -23,12 +26,21 @@ module.exports = {
},

ERROR_MESSAGE,
ERROR_MESSAGE_TEMPLATE_TAG,

create(context) {
return {
ClassDeclaration(node) {
if (isGlimmerComponent(context, node) && !node.decorators && node.body.body.length === 0) {
const nodeIsGlimmerComponent = isGlimmerComponent(context, node);
if (nodeIsGlimmerComponent && !node.decorators && node.body.body.length === 0) {
context.report({ node, message: ERROR_MESSAGE });
} else if (
nodeIsGlimmerComponent &&
node.body.body.length === 1 &&
isClassPropertyOrPropertyDefinition(node.body.body[0]) &&
node.body.body[0].key?.callee?.name === '__GLIMMER_TEMPLATE'
) {
context.report({ node, message: ERROR_MESSAGE_TEMPLATE_TAG });
}
},
};
Expand Down
28 changes: 28 additions & 0 deletions tests/lib/rules-preprocessor/gjs-gts-processor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ const valid = [
</template>
`,
},
{
filename: 'my-component.gjs',
code: `
import Component from '@glimmer/component';
export default class MyComponent extends Component {
foo() {
return this.args.bar + this.args.baz;
}
<template>Hello World!</template>
}
`,
},
/**
* TODO: SKIP this scenario. Tracked in https://github.com/ember-cli/eslint-plugin-ember/issues/1685
{
Expand All @@ -102,6 +115,21 @@ const valid = [
];

const invalid = [
{
filename: 'my-component.gjs',
code: `import Component from '@glimmer/component';
export default class Chris extends Component {
<template>Hello!</template>
}`,
errors: [
{
message: 'Do not create empty backing classes for Glimmer template tag only components.',
line: 2,
column: 20,
endColumn: 6,
},
],
},
{
filename: 'my-component.gjs',
code: `
Expand Down

0 comments on commit 55b987c

Please sign in to comment.