chore: new eslint rule ban-context-export

change/@fluentui-eslint-plugin-bae0af9c-456e-420c-9560-08c80175e2de.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "chore: new eslint rule: `ban-context-export`",
+  "packageName": "@fluentui/eslint-plugin",
+  "email": "lingfangao@hotmail.com",
+  "dependentChangeType": "none"
+}

change/@fluentui-react-context-selector-e5b506ef-63ca-4690-8e76-eac9690a95f6.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "packageName": "@fluentui/react-context-selector",
+  "comment": "Add eslint ignore for context export",
+  "email": "lingfangao@hotmail.com",
+  "dependentChangeType": "none"
+}

change/@fluentui-react-utilities-5fa1f06a-3abd-457a-b2b4-0dd403f861fd.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Add eslint-ignore",
+  "packageName": "@fluentui/react-utilities",
+  "email": "lingfangao@hotmail.com",
+  "dependentChangeType": "none"
+}

packages/eslint-plugin/README.md

@@ -17,6 +17,36 @@ Helpers for customizing configuration are exported under a `configHelpers` objec
 
 ## Rules
 
+### `ban-context-export`
+
+Exporting context objects as a part of the public API can lead to unexpected usages of context by customers and might
+impede future refactoring. To allow customers use context while encapsulating our internals correctly, the developer
+should export a provider and hook.
+
+**❌ Don't**
+
+```ts
+// src/context.ts
+import * as React from 'react';
+export const MyContext = React.createContext();
+
+// src/index.ts
+export { MyContext } from './context';
+```
+
+**✅ Do**
+
+```ts
+// src/context.ts
+import * as React from 'react';
+const MyContext = React.createContext();
+export const MyContextProvider = MyContext.Provider;
+export const useMyContext = () => React.useContext(MyContext);
+
+// src/index.ts
+export { MyContextProvider, useMyContext } from './context';
+```
+
 ### `ban-imports`
 
 Ban importing or re-exporting from certain paths or modules. You can either ban the entire path, or only certain names. (Inspired by TSLint's [`import-blacklist`](https://palantir.github.io/tslint/rules/import-blacklist/).)

packages/eslint-plugin/package.json

@@ -9,7 +9,8 @@
   },
   "license": "MIT",
   "scripts": {
-    "lint": "tsc --noEmit && eslint --ext .js --cache ."
+    "lint": "tsc --noEmit && eslint --ext .js --cache .",
+    "test": "yarn jest --passWithNoTests"
   },
   "dependencies": {
     "@rnx-kit/eslint-plugin": "^0.2.5",
@@ -26,6 +27,7 @@
     "eslint-plugin-jsx-a11y": "^6.4.1",
     "eslint-plugin-react": "^7.24.0",
     "eslint-plugin-react-hooks": "^4.2.0",
+    "minimatch": "^3.0.4",
     "jju": "^1.4.0"
   },
   "peerDependencies": {

packages/eslint-plugin/src/configs/base.js

@@ -1,9 +1,15 @@
 // @ts-check
 
 const path = require('path');
+const configHelpers = require('../utils/configHelpers');
 
 const { getNamingConventionRule } = require('../utils/configHelpers');
 
+/** @type {import("eslint").Linter.RulesRecord} */
+const typeAwareRules = {
+  '@fluentui/ban-context-export': ['error', { exclude: ['**/react-shared-contexts/**'] }],
+};
+
 /** @type {import("eslint").Linter.Config} */
 module.exports = {
   extends: [path.join(__dirname, 'core')],
@@ -15,6 +21,8 @@ module.exports = {
     ...getNamingConventionRule(),
   },
   overrides: [
+    // Enable rules requiring type info only for appropriate files/circumstances
+    ...configHelpers.getTypeInfoRuleOverrides(typeAwareRules),
     {
       files: '**/src/index.{ts,tsx,js}',
       rules: {

packages/eslint-plugin/src/index.js

@@ -10,6 +10,7 @@ module.exports = {
 
   rules: {
     'ban-imports': require('./rules/ban-imports'),
+    'ban-context-export': require('./rules/ban-context-export'),
     'deprecated-keyboard-event-props': require('./rules/deprecated-keyboard-event-props'),
     'max-len': require('./rules/max-len'),
     'no-global-react': require('./rules/no-global-react'),

packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/context.ts

@@ -0,0 +1,4 @@
+// eslint-disable-next-line import/no-extraneous-dependencies
+import { createContext } from '@fluentui/react-context-selector';
+
+export const MyContext = createContext({});

packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/special-path/src/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/context.ts

@@ -0,0 +1,4 @@
+// eslint-disable-next-line import/no-extraneous-dependencies
+import * as React from 'react';
+
+export const MyContext = React.createContext({});

packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/context.ts

@@ -0,0 +1,4 @@
+// eslint-disable-next-line import/no-extraneous-dependencies
+import * as React from 'react';
+
+export const MyContext = React.createContext({});

packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/src/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/context.ts

@@ -0,0 +1,2 @@
+// eslint-disable-next-line import/no-extraneous-dependencies
+export const MyContext = {};

packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/index.ts

@@ -0,0 +1,2 @@
+// This file needs to be in the file system for tests to run
+// https://typescript-eslint.io/docs/development/custom-rules/#testing-typed-rules

packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "extends": "../../../../../../../tsconfig.base.json",
+  "compilerOptions": {
+    "noEmit": false,
+    "lib": ["ES2019", "dom"],
+    "outDir": "dist",
+    "declaration": true,
+    "declarationDir": "dist/types",
+    "types": ["static-assets", "environment"]
+  }
+}

packages/eslint-plugin/src/rules/ban-context-export/index.js

@@ -0,0 +1,135 @@
+// @ts-check
+const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/experimental-utils');
+const createRule = require('../../utils/createRule');
+const minimatch = require('minimatch');
+
+/** @typedef { import('@typescript-eslint/experimental-utils').TSESTree.VariableDeclarator } VariableDeclarator*/
+/** @typedef { import('@typescript-eslint/experimental-utils').TSESTree.ExportSpecifier} ExportSpecifier */
+/**
+ * @typedef {{
+ *  exclude?: string[];
+ * }} Options
+ */
+
+/** @type {Options} */
+const defaultOptions = {};
+
+module.exports = createRule({
+  name: 'ban-context-export',
+  defaultOptions: [],
+  meta: {
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          exclude: {
+            type: 'array',
+            items: {
+              type: 'string',
+            },
+            description: 'List of files to exclude',
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
+    type: 'problem',
+    docs: {
+      description: 'Ban export of React context or context selector objects',
+      category: 'Best Practices',
+      recommended: 'error',
+    },
+    messages: {
+      nativeContext: '{{exportName}} should not be exported directly',
+      contextSelector: '{{exportName}} should not be exported directly',
+    },
+  },
+  create(context) {
+    const rawOptions = /** @type {Options[]} */ (context.options);
+    const { exclude = [] } = rawOptions.length ? rawOptions[0] : defaultOptions;
+    const { program, esTreeNodeToTSNodeMap } = ESLintUtils.getParserServices(context);
+    /** @type {import("typescript").TypeChecker | undefined} */
+    let typeChecker;
+
+    /**
+     * @param { ExportSpecifier | VariableDeclarator } node
+     * @param {string} exportName
+     */
+    function checkContextType(node, exportName) {
+      const currentFileName = context.getFilename();
+      if (exclude.some(pattern => minimatch(currentFileName, pattern))) {
+        return;
+      }
+
+      const isTopLevelExport = currentFileName.endsWith('src/index.ts');
+      if (!isTopLevelExport) {
+        return;
+      }
+
+      if (!typeChecker) {
+        typeChecker = program.getTypeChecker();
+      }
+
+      const tsNode = esTreeNodeToTSNodeMap.get(node);
+      const typeNode = typeChecker.getTypeAtLocation(tsNode);
+
+      // @fluentui/react-context-selector
+      if (typeNode.aliasSymbol?.name === 'Context' && typeNode.aliasSymbol.declarations?.length) {
+        const firstDeclaration = typeNode.aliasSymbol.declarations[0];
+        const fileName = firstDeclaration.parent.getSourceFile().fileName;
+        if (fileName.includes('react-context-selector')) {
+          context.report({
+            node,
+            messageId: 'contextSelector',
+            data: {
+              exportName,
+              filename: context.getFilename(),
+            },
+          });
+        }
+      }
+
+      // Native react
+      if (typeNode.symbol?.name === 'Context' && typeNode.symbol.declarations?.length) {
+        const firstDeclaration = typeNode.symbol.declarations[0];
+        const fileName = firstDeclaration.parent.getSourceFile().fileName;
+        if (/\/node_modules\/@types\/react\//.test(fileName)) {
+          context.report({
+            node,
+            messageId: 'nativeContext',
+            data: {
+              exportName,
+              filename: context.getFilename(),
+            },
+          });
+        }
+      }
+    }
+
+    return {
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      ExportNamedDeclaration(exportNamedDeclaration) {
+        if (exportNamedDeclaration.declaration?.type === AST_NODE_TYPES.VariableDeclaration) {
+          /** @type { import('@typescript-eslint/experimental-utils').TSESTree.VariableDeclaration } */
+          const variableDeclaration = exportNamedDeclaration.declaration;
+          variableDeclaration.declarations.forEach(declaration => {
+            let identifierName = 'unknown';
+            if (declaration.id.type === AST_NODE_TYPES.Identifier) {
+              /** @type { import('@typescript-eslint/experimental-utils').TSESTree.Identifier } */
+              const identifier = declaration.id;
+              identifierName = identifier.name;
+            }
+
+            checkContextType(declaration, identifierName);
+          });
+        }
+      },
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      ExportSpecifier(exportSpecifier) {
+        if (exportSpecifier.exported.name.includes('Context')) {
+          checkContextType(exportSpecifier, exportSpecifier.exported.name);
+        }
+      },
+    };
+  },
+});