Bugfix: no-empty-glimmer-component-classes to allow generic type subc…

…lass (#1876)
ember-cli · May 23, 2023 · 9e2f2ae · 9e2f2ae
1 parent d779c62
commit 9e2f2ae
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 14 deletions.
diff --git 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 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.
+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. An exception to this case is if you need the component to be generic over part of its type signature.
 
 To fix violations of this rule:
 
@@ -34,6 +34,14 @@ export default class MyComponent extends Component {
 }
 ```
 
+```ts
+import Component from '@glimmer/component';
+
+export interface TypeSig {}
+
+export default class MyComponent extends Component<TypeSig> {}
+```
+
 Examples of **correct** code for this rule:
 
 ```js
@@ -74,6 +82,15 @@ export default class MyComponent extends Component {
 }
 ```
 
+```ts
+import Component from '@glimmer/component';
+
+export interface SomeSig {}
+export interface SomeOtherSig {}
+
+export default class MyComponent<SomeSig> extends Component<SomeOtherSig> {}
+```
+
 ## References
 
 - [Glimmer Components - Octane Upgrade Guide](https://guides.emberjs.com/release/upgrading/current-edition/glimmer-components/)

diff --git a/lib/rules/no-empty-glimmer-component-classes.js b/lib/rules/no-empty-glimmer-component-classes.js
@@ -31,14 +31,18 @@ module.exports = {
   create(context) {
     return {
       ClassDeclaration(node) {
-        const nodeIsGlimmerComponent = isGlimmerComponent(context, node);
-        if (nodeIsGlimmerComponent && !node.decorators && node.body.body.length === 0) {
+        if (!isGlimmerComponent(context, node)) {
+          return;
+        }
+
+        const subClassHasTypeDefinition = node.typeParameters?.params?.length > 0;
+        if (!node.decorators && node.body.body.length === 0 && !subClassHasTypeDefinition) {
           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'
+          node.body.body[0].key?.callee?.name === '__GLIMMER_TEMPLATE' &&
+          !subClassHasTypeDefinition
         ) {
           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
@@ -13,10 +13,10 @@ const plugin = require('../../../lib');
 /**
  * Helper function which creates ESLint instance with enabled/disabled autofix feature.
  *
- * @param {CLIEngineOptions} [options={}] Whether to enable autofix feature.
+ * @param {String} parser The parser to use.
  * @returns {ESLint} ESLint instance to execute in tests.
  */
-function initESLint(options) {
+function initESLint(parser = '@babel/eslint-parser') {
   // tests must be run with ESLint 7+
   return new ESLint({
     ignore: false,
@@ -27,7 +27,7 @@ function initESLint(options) {
       env: {
         browser: true,
       },
-      parser: '@babel/eslint-parser',
+      parser,
       parserOptions: {
         ecmaVersion: 2020,
         sourceType: 'module',
@@ -45,7 +45,6 @@ function initESLint(options) {
         'ember/no-array-prototype-extensions': 'error',
       },
     },
-    ...options,
   });
 }
 
@@ -99,6 +98,24 @@ const valid = [
       }
     `,
   },
+  {
+    filename: 'my-component.gts',
+    code: `import Component from '@glimmer/component';
+
+    interface ListSignature<T> {
+      Args: {
+        items: Array<T>;
+      };
+      Blocks: {
+        default: [item: T]
+      };
+    }
+
+    export default class List<T> extends Component<ListSignature<T>> {
+      <template>Hello!</template>
+    }`,
+    parser: '@typescript-eslint/parser',
+  },
   /**
    * TODO: SKIP this scenario. Tracked in https://github.com/ember-cli/eslint-plugin-ember/issues/1685
   {
@@ -118,7 +135,7 @@ const invalid = [
   {
     filename: 'my-component.gjs',
     code: `import Component from '@glimmer/component';
-    export default class Chris extends Component {
+    export default class MyComponent extends Component {
       <template>Hello!</template>
     }`,
     errors: [
@@ -130,6 +147,7 @@ const invalid = [
       },
     ],
   },
+
   {
     filename: 'my-component.gjs',
     code: `
@@ -272,11 +290,11 @@ const invalid = [
 describe('template-vars', () => {
   describe('valid', () => {
     for (const scenario of valid) {
-      const { code, filename } = scenario;
+      const { code, filename, parser } = scenario;
 
       // eslint-disable-next-line jest/valid-title
       it(code, async () => {
-        const eslint = initESLint();
+        const eslint = initESLint(parser);
         const results = await eslint.lintText(code, { filePath: filename });
         const resultErrors = results.flatMap((result) => result.messages);
 
@@ -296,11 +314,11 @@ describe('template-vars', () => {
 
   describe('invalid', () => {
     for (const scenario of invalid) {
-      const { code, filename, errors } = scenario;
+      const { code, filename, errors, parser } = scenario;
 
       // eslint-disable-next-line jest/valid-title
       it(code, async () => {
-        const eslint = initESLint();
+        const eslint = initESLint(parser);
         const results = await eslint.lintText(code, { filePath: filename });
 
         const resultErrors = results.flatMap((result) => result.messages);

diff --git a/tests/lib/rules/no-empty-glimmer-component-classes.js b/tests/lib/rules/no-empty-glimmer-component-classes.js
@@ -34,6 +34,34 @@ ruleTester.run('no-empty-glimmer-component-classes', rule, {
 
     @MyDecorator
     class MyComponent extends Component {}`,
+    {
+      code: `
+      import Component from '@glimmer/component';
+
+      interface ListSignature<T> {
+        Args: {
+          items: Array<T>;
+        };
+        Blocks: {
+          default: [item: T]
+        };
+      }
+
+      export default class List<T> extends Component<ListSignature<T>> {}
+      `,
+      parser: require.resolve('@typescript-eslint/parser'),
+    },
+    {
+      code: `
+        import Component from '@glimmer/component';
+
+        export interface SomeSig {}
+        export interface SomeOtherSig {}
+
+        export default class MyComponent<SomeSig> extends Component<SomeOtherSig> {}
+      `,
+      parser: require.resolve('@typescript-eslint/parser'),
+    },
   ],
   invalid: [
     {
@@ -50,5 +78,17 @@ ruleTester.run('no-empty-glimmer-component-classes', rule, {
       output: null,
       errors: [{ message: ERROR_MESSAGE, type: 'ClassDeclaration' }],
     },
+    {
+      code: `
+      import Component from '@glimmer/component';
+
+      export interface TypeSig {}
+
+      export default class MyComponent extends Component<TypeSig> {}
+      `,
+      output: null,
+      parser: require.resolve('@typescript-eslint/parser'),
+      errors: [{ message: ERROR_MESSAGE, type: 'ClassDeclaration' }],
+    },
   ],
 });