Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for class only having template tag in no-empty-glimmer-component-classes rule #1866

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 22 additions & 1 deletion docs/rules/no-empty-glimmer-component-classes.md
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,14 @@ import Component from '@glimmer/component';
class MyComponent extends Component {}
```

```js
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 +62,20 @@ import MyDecorator from 'my-decorator';
class MyComponent extends Component {}
```

```js
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
@@ -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
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