feat(eslint-plugin): add top-level-styles eslint rule (#595)

* Add top-level-styles eslint rule

* Change files

* code review feedback

change/@griffel-eslint-plugin-dd20b1e1-c272-409a-95f9-19ca824187ce.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "feat: add top-level-styles eslint rule",
+  "packageName": "@griffel/eslint-plugin",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/eslint-plugin/README.md

@@ -23,7 +23,7 @@ Add `@griffel` to the plugins section and `plugin:@griffel/recommended` to the e
 
 ## Shareable configurations
 
-This plugin exports recommended configuration that enforce good practices, but you can use only uses that are required for your use case:
+This plugin exports recommended configuration that enforce good practices, but you can choose to use only the ones that are necessary for your use case:
 
 ```json
 {
@@ -47,5 +47,6 @@ You can find more info about enabled rules in the [Supported Rules section](#sup
 | [`@griffel/hook-naming`](./src/rules/hook-naming.md)                                       | Ensure that hooks returned by the `makeStyles()` function start with "use"                                      | ❌  |
 | [`@griffel/no-invalid-shorthand-arguments`](./src/rules/no-invalid-shorthand-arguments.md) | All shorthands must not use space separators, and instead pass in multiple arguments                            | ✅  |
 | [`@griffel/no-shorthands`](./src/rules/no-shorthands.md)                                   | Enforce usage of CSS longhands                                                                                  | ✅  |
-| [`@griffel/styles-file`](./src/rules/styles-file.md)                                       | Ensures that all `makeStyles()` and `makeResetStyles()` calls are placed in a `.styles.js` or `.styles.ts` file | ❌  |
+| [`@griffel/styles-file`](./src/rules/styles-file.md)                                       | Ensures that all `makeStyles()` and `makeResetStyles()` calls are placed in a `.styles.js` or `.styles.ts` file | ❌  | limitations) | ❌ |
 | [`@griffel/pseudo-element-naming`](./src/rules/pseudo-element-naming.md)                   | Ensures that all Pseudo Elements start with two colons                                                          | ✅  |
+| [`@griffel/top-level-styles`](./src/rules/top-level-styles.md)                    | Ensures that all `makeStyles()`, `makeResetStyles()` and `makeStaticStyles()` calls are written in the top level of a file to discourage developer misuse.  | ❌  |

packages/eslint-plugin/src/rules/styles-file.ts

@@ -1,46 +1,14 @@
 import { ESLintUtils } from '@typescript-eslint/utils';
-import type { TSESTree } from '@typescript-eslint/utils';
 
-import { isMakeStylesCallExpression } from '../utils/helpers';
+import { isMakeStylesCallExpression, isMakeStylesImport } from '../utils/helpers';
 import { getDocsUrl } from '../utils/getDocsUrl';
 
-const MATCHING_PACKAGES = new Set(['@fluentui/react-components', '@griffel/core', '@griffel/react']);
 const STYLES_FILE_PATTERN = /^.*\.(styles)\.[j|t]s$/;
 
-function isMatchingPackage(packageName: string) {
-  return MATCHING_PACKAGES.has(packageName);
-}
-
 function isStylesFile(fileName: string) {
   return STYLES_FILE_PATTERN.test(fileName);
 }
 
-/**
- * @param node - import node from AST
- * @returns true if makeStyles is imported, or if the entire library is imported. Otherwise returns false
- */
-function isMakeStylesImport(node: TSESTree.ImportDeclaration) {
-  return (
-    isMatchingPackage(node.source.value) &&
-    node.specifiers.filter(specifier => {
-      // import * as ... from
-      if (specifier.type === 'ImportNamespaceSpecifier') {
-        return true;
-      }
-
-      if ('imported' in specifier) {
-        return (
-          specifier.imported.name === 'makeStyles' || // import { makeStyles } from
-          specifier.imported.name === 'makeStaticStyles' || // import { makeStaticStyles } from
-          specifier.imported.name === 'makeResetStyles' // import { makeResetStyles } from
-        );
-      }
-
-      return false;
-    }).length > 0
-  );
-}
-
 export const RULE_NAME = 'styles-file';
 
 export const stylesFileRule = ESLintUtils.RuleCreator(getDocsUrl)({

packages/eslint-plugin/src/rules/top-level-styles.md

@@ -0,0 +1,62 @@
+# Ensure `makeStyles`, `makeResetStyles` and `makeStaticStyles` are only called at the top-level of a file
+
+This rule discourages developers from misusing the `makeStyles`, `makeResetStyles` and `makeStaticStyles` methods. These methods don't allow passing runtime values for constructing the styles, as described here under [limitations](https://griffel.js.org/react/guides/). To encourage proper usage, this rule only permits calling those methods in the _top-level scope_ of a file, where it's far less likely developers will pass runtime values.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+// filename: component.tsx
+import { makeStyles } from '@griffel/react';
+
+export const getStyles = () => {
+  const useStyles = makeStyles({
+    root: {
+      backgroundColor: 'red',
+    },
+  });
+
+  return useStyles;
+}
+```
+
+```js
+// filename: component.tsx
+import { makeStyles } from '@fluentui/react-components';
+
+export class MyClass {
+  getStyles () {
+    const styles = makeStyles({
+      root: {
+        backgroundColor: 'red',
+      },
+    });
+
+    return styles;
+  }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+import { makeStyles } from '@griffel/react';
+
+export const useStyles = makeStyles({
+  root: {
+    backgroundColor: 'red',
+  },
+});
+```
+
+```js
+import { makeStyles } from '@fluentui/react-components';
+import { generateStyles } from './custom-css';
+
+export const useStyles = generateStyles(makeStyles({
+  root: {
+    backgroundColor: 'red',
+  },
+}));
+```

packages/eslint-plugin/src/rules/top-level-styles.test.ts

@@ -0,0 +1,116 @@
+import { TSESLint } from '@typescript-eslint/utils';
+import { stylesFileRule, RULE_NAME } from './top-level-styles';
+import * as path from 'path';
+
+const componentFileName = 'packages/components/components-foo/foo.tsx';
+
+const ruleTester = new TSESLint.RuleTester({
+  parser: path.resolve('./node_modules/@typescript-eslint/parser/dist'),
+  parserOptions: {
+    ecmaVersion: 2018,
+    sourceType: 'module',
+  },
+});
+
+ruleTester.run(RULE_NAME, stylesFileRule, {
+  valid: [
+    {
+      code: `
+import { makeStyles } from '@fluentui/react-components';
+
+export const useStyles = makeStyles({
+  root: { color: 'blue' },
+});
+`,
+      filename: componentFileName,
+    },
+    {
+      code: `
+import { makeStyles } from 'unknown-package';
+
+export function getStyles() {
+  const useStyles = makeStyles({
+    root: { color: 'blue' },
+  });
+
+  return useStyles;
+}
+`,
+      filename: componentFileName,
+    },
+  ],
+
+  invalid: [
+    {
+      code: `
+        import { makeStyles } from '@fluentui/react-components';
+        function getStyles() {
+          const useStyles = makeStyles({
+            root: { color: 'blue' },
+          });
+
+          return useStyles;
+        }
+      `,
+      errors: [{ messageId: 'foundInvalidUsage', data: { methodName: 'makeStyles' } }],
+      filename: componentFileName,
+    },
+    {
+      code: `
+        import { makeStaticStyles } from '@fluentui/react-components';
+        function getStyles() {
+          const useStyles = makeStaticStyles({
+            root: { color: 'blue' },
+          });
+
+          return useStyles;
+        }
+      `,
+      errors: [{ messageId: 'foundInvalidUsage', data: { methodName: 'makeStaticStyles' } }],
+      filename: componentFileName,
+    },
+    {
+      code: `
+        import { makeResetStyles } from '@fluentui/react-components';
+        function getStyles() {
+          const useStyles = makeResetStyles({
+            root: { color: 'blue' },
+          });
+
+          return useStyles;
+        }
+      `,
+      errors: [{ messageId: 'foundInvalidUsage', data: { methodName: 'makeResetStyles' } }],
+      filename: componentFileName,
+    },
+    {
+      code: `
+        import { makeStyles } from '@griffel/react';
+        const getStyles = () => {
+          const useStyles = makeStyles({
+            root: { color: 'blue' },
+          });
+
+          return useStyles;
+        };
+      `,
+      errors: [{ messageId: 'foundInvalidUsage', data: { methodName: 'makeStyles' } }],
+      filename: componentFileName,
+    },
+    {
+      code: `
+        import { makeStyles } from '@fluentui/react-components';
+
+        class MyClass {
+          constructor() {
+            const useStyles = makeStyles({
+              root: { color: 'blue' },
+            });
+          }
+        }
+      `,
+      errors: [{ messageId: 'foundInvalidUsage', data: { methodName: 'makeStyles' } }],
+      filename: componentFileName,
+    },
+  ],
+});

packages/eslint-plugin/src/rules/top-level-styles.ts

@@ -0,0 +1,51 @@
+import { ESLintUtils } from '@typescript-eslint/utils';
+
+import { getMakeStylesCallExpression, isMakeStylesImport } from '../utils/helpers';
+import { getDocsUrl } from '../utils/getDocsUrl';
+
+export const RULE_NAME = 'top-level-styles';
+
+export const stylesFileRule = ESLintUtils.RuleCreator(getDocsUrl)({
+  name: RULE_NAME,
+  meta: {
+    docs: {
+      description:
+        'Ensure makeStyles(), makeResetStyles() and makeStaticStyles() calls are placed in the top level of the file',
+      recommended: 'recommended',
+    },
+    messages: {
+      foundInvalidUsage: '`{{ methodName }}` should be only be called from the top-level of the file',
+    },
+    type: 'problem',
+    schema: [],
+  },
+  defaultOptions: [],
+  create(context) {
+    let isMakeStylesImported = false;
+
+    return {
+      ImportDeclaration(node) {
+        if (!isMakeStylesImported) {
+          isMakeStylesImported = isMakeStylesImport(node);
+        }
+      },
+
+      'FunctionDeclaration CallExpression, ArrowFunctionExpression CallExpression, FunctionExpression CallExpression':
+        function (node) {
+          if (!isMakeStylesImported) {
+            return;
+          }
+          const methodName = getMakeStylesCallExpression(node, 'makeStyles', 'makeStaticStyles', 'makeResetStyles');
+          if (methodName) {
+            context.report({
+              messageId: 'foundInvalidUsage',
+              data: {
+                methodName,
+              },
+              node,
+            });
+          }
+        },
+    };
+  },
+});

packages/eslint-plugin/src/utils/helpers.ts

@@ -34,20 +34,62 @@ export function isMakeStylesCallExpression(
   node: TSESTree.CallExpression,
   ...functionNames: [MakeStylesName, ...MakeStylesName[]]
 ): boolean {
+  return getMakeStylesCallExpression(node, ...functionNames) !== null;
+}
+
+export function getMakeStylesCallExpression(
+  node: TSESTree.CallExpression,
+  ...functionNames: [MakeStylesName, ...MakeStylesName[]]
+) {
   const { callee } = node;
   const names: ReadonlySet<string> = new Set(functionNames);
 
-  if (isIdentifier(callee)) {
+  if (isIdentifier(callee) && names.has(callee.name)) {
     // makeStyles({})
-    return names.has(callee.name);
+    return callee.name;
   } else if (isMemberExpression(callee)) {
-    return (
-      // something.makeStyles({})
-      (isIdentifier(callee.property) && names.has(callee.property.name)) ||
-      // something['makeStyles']({})
-      (isStringLiteral(callee.property) && names.has(callee.property.value))
-    );
-  } else {
-    return false;
+    // something.makeStyles({})
+    if (isIdentifier(callee.property) && names.has(callee.property.name)) {
+      return callee.property.name;
+    }
+
+    // something['makeStyles']({})
+    if (isStringLiteral(callee.property) && names.has(callee.property.value)) {
+      return callee.property.value;
+    }
   }
+
+  return null;
+}
+
+const MATCHING_PACKAGES = new Set(['@fluentui/react-components', '@griffel/core', '@griffel/react']);
+
+function isMatchingPackage(packageName: string) {
+  return MATCHING_PACKAGES.has(packageName);
+}
+
+/**
+ * @param node - import node from AST
+ * @returns true if makeStyles is imported, or if the entire library is imported. Otherwise returns false
+ */
+export function isMakeStylesImport(node: TSESTree.ImportDeclaration) {
+  return (
+    isMatchingPackage(node.source.value) &&
+    node.specifiers.filter(specifier => {
+      // import * as ... from
+      if (specifier.type === 'ImportNamespaceSpecifier') {
+        return true;
+      }
+
+      if ('imported' in specifier) {
+        return (
+          specifier.imported.name === 'makeStyles' || // import { makeStyles } from
+          specifier.imported.name === 'makeStaticStyles' || // import { makeStaticStyles } from
+          specifier.imported.name === 'makeResetStyles' // import { makeResetStyles } from
+        );
+      }
+
+      return false;
+    }).length > 0
+  );
 }