From 55b987c6469d969051c774e0ae1a2ae67c39bc84 Mon Sep 17 00:00:00 2001 From: Chris Ng Date: Thu, 18 May 2023 22:30:55 -0400 Subject: [PATCH] Account for only having template tag in `no-empty-glimmer-component-classes` rule (#1866) --- .../no-empty-glimmer-component-classes.md | 25 ++++++++++++++++- .../no-empty-glimmer-component-classes.js | 14 +++++++++- .../gjs-gts-processor-test.js | 28 +++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-empty-glimmer-component-classes.md b/docs/rules/no-empty-glimmer-component-classes.md index 2a9d87a688..f0bdcc7926 100644 --- a/docs/rules/no-empty-glimmer-component-classes.md +++ b/docs/rules/no-empty-glimmer-component-classes.md @@ -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: @@ -26,6 +26,15 @@ import Component from '@glimmer/component'; class MyComponent extends Component {} ``` + +``` +import Component from '@glimmer/component'; + +export default class MyComponent extends Component { + +} +``` + Examples of **correct** code for this rule: ```js @@ -54,7 +63,21 @@ import MyDecorator from 'my-decorator'; class MyComponent extends Component {} ``` + +``` +import Component from '@glimmer/component'; + +export default class MyComponent extends Component { + foo() { + return this.args.bar + this.args.baz; + } + + +} +``` + ## 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/) diff --git a/lib/rules/no-empty-glimmer-component-classes.js b/lib/rules/no-empty-glimmer-component-classes.js index b21a9443a8..34f0df5e5f 100644 --- a/lib/rules/no-empty-glimmer-component-classes.js +++ b/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 @@ -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 }); } }, }; diff --git a/tests/lib/rules-preprocessor/gjs-gts-processor-test.js b/tests/lib/rules-preprocessor/gjs-gts-processor-test.js index 6fb7f5b56f..7866fe698d 100644 --- a/tests/lib/rules-preprocessor/gjs-gts-processor-test.js +++ b/tests/lib/rules-preprocessor/gjs-gts-processor-test.js @@ -86,6 +86,19 @@ const valid = [ `, }, + { + filename: 'my-component.gjs', + code: ` + import Component from '@glimmer/component'; + export default class MyComponent extends Component { + foo() { + return this.args.bar + this.args.baz; + } + + + } + `, + }, /** * TODO: SKIP this scenario. Tracked in https://github.com/ember-cli/eslint-plugin-ember/issues/1685 { @@ -102,6 +115,21 @@ const valid = [ ]; const invalid = [ + { + filename: 'my-component.gjs', + code: `import Component from '@glimmer/component'; + export default class Chris extends Component { + + }`, + 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: `