From 7fc669b2f21d4bb020ff12322653dcd6b55c8456 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 14:31:16 +0200 Subject: [PATCH 01/18] fix: Syncpack after releasing v9 --- apps/public-docsite-v9/package.json | 4 ++-- packages/fluentui/react-northstar/package.json | 2 +- packages/react/package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/public-docsite-v9/package.json b/apps/public-docsite-v9/package.json index 4520671109e70..262bf4e48ca8b 100644 --- a/apps/public-docsite-v9/package.json +++ b/apps/public-docsite-v9/package.json @@ -23,9 +23,9 @@ "@fluentui/react": "^8.68.4", "@fluentui/scripts": "^1.0.0", "@fluentui/storybook": "^1.0.0", - "@fluentui/react-components": "^9.0.0-rc.10", + "@fluentui/react-components": "^9.0.0-rc.11", "@fluentui/react-storybook-addon": "9.0.0-rc.1", - "@fluentui/react-theme": "9.0.0-rc.7", + "@fluentui/react-theme": "9.0.0-rc.8", "@griffel/react": "1.0.5", "react": "16.14.0", "react-dom": "16.14.0", diff --git a/packages/fluentui/react-northstar/package.json b/packages/fluentui/react-northstar/package.json index 85438d7449c4e..78ccd5039661c 100644 --- a/packages/fluentui/react-northstar/package.json +++ b/packages/fluentui/react-northstar/package.json @@ -14,7 +14,7 @@ "@fluentui/react-component-ref": "^0.63.0", "@fluentui/react-icons-northstar": "^0.63.0", "@fluentui/react-northstar-styles-renderer": "^0.63.0", - "@fluentui/react-portal-compat-context": "^9.0.0-rc.1", + "@fluentui/react-portal-compat-context": "^9.0.0-rc.2", "@fluentui/react-proptypes": "^0.63.0", "@fluentui/state": "^0.63.0", "@fluentui/styles": "^0.63.0", diff --git a/packages/react/package.json b/packages/react/package.json index 9756d3c7ea5c7..8d7fedba3b9de 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -49,7 +49,7 @@ "@fluentui/merge-styles": "^8.5.2", "@fluentui/react-focus": "^8.6.0", "@fluentui/react-hooks": "^8.5.5", - "@fluentui/react-portal-compat-context": "^9.0.0-rc.1", + "@fluentui/react-portal-compat-context": "^9.0.0-rc.2", "@fluentui/react-window-provider": "^2.2.1", "@fluentui/set-version": "^8.2.1", "@fluentui/style-utilities": "^8.6.7", From b93d830d5d85023fecef6f991564994a86d150d5 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 17:03:55 +0200 Subject: [PATCH 02/18] chore: new eslint rule: `ban-context-export` This PR introduces a new eslint rule to ban exporting context objects from Fluent UI [you can read more detailed documentation here]() --- packages/eslint-plugin/README.md | 30 +++++ packages/eslint-plugin/src/configs/core.js | 1 + packages/eslint-plugin/src/index.js | 1 + .../fixtures/context-selector/src/context.ts | 4 + .../fixtures/context-selector/src/index.ts | 2 + .../fixtures/context-selector/tsconfig.json | 11 ++ .../fixtures/export-specifier/src/context.ts | 4 + .../fixtures/export-specifier/src/index.ts | 2 + .../fixtures/export-specifier/tsconfig.json | 11 ++ .../internal-export/src/internal/context.ts | 4 + .../internal-export/src/internal/index.ts | 2 + .../fixtures/internal-export/tsconfig.json | 11 ++ .../fixtures/named-export/src/index.ts | 2 + .../fixtures/named-export/tsconfig.json | 11 ++ .../fixtures/not-a-context/src/context.ts | 2 + .../fixtures/not-a-context/src/index.ts | 2 + .../fixtures/not-a-context/tsconfig.json | 11 ++ .../src/rules/ban-context-export/index.js | 105 ++++++++++++++++++ .../rules/ban-context-export/index.test.js | 77 +++++++++++++ 19 files changed, 293 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/context.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/tsconfig.json create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/context.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/tsconfig.json create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/context.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/tsconfig.json create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/src/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/tsconfig.json create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/context.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/tsconfig.json create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/index.js create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/index.test.js diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 71aac12a6f9d5..10231018fcf4d 100644 --- a/packages/eslint-plugin/README.md +++ b/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/).) diff --git a/packages/eslint-plugin/src/configs/core.js b/packages/eslint-plugin/src/configs/core.js index b6a69150c2b9b..cbd958cf7f02d 100644 --- a/packages/eslint-plugin/src/configs/core.js +++ b/packages/eslint-plugin/src/configs/core.js @@ -238,6 +238,7 @@ const typeAwareRules = { * plugin: https://github.com/gund/eslint-plugin-deprecation */ 'deprecation/deprecation': 'error', + '@fluentui/ban-context-export': 'error', }; /** diff --git a/packages/eslint-plugin/src/index.js b/packages/eslint-plugin/src/index.js index 033c21c969dc3..3849051a74318 100644 --- a/packages/eslint-plugin/src/index.js +++ b/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'), diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/context.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/context.ts new file mode 100644 index 0000000000000..1be6d70682d0c --- /dev/null +++ b/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({}); diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/src/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/context-selector/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/context.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/context.ts new file mode 100644 index 0000000000000..fad576eb75a42 --- /dev/null +++ b/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({}); diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/src/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/export-specifier/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/context.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/context.ts new file mode 100644 index 0000000000000..fad576eb75a42 --- /dev/null +++ b/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({}); diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/src/internal/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/internal-export/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/src/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/src/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/named-export/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/context.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/context.ts new file mode 100644 index 0000000000000..5da144db61400 --- /dev/null +++ b/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 = {}; diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/src/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/not-a-context/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js new file mode 100644 index 0000000000000..b892358195861 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -0,0 +1,105 @@ +// @ts-check +const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/experimental-utils'); +const createRule = require('../../utils/createRule'); + +/** @typedef { import('@typescript-eslint/experimental-utils').TSESTree.VariableDeclarator } VariableDeclarator*/ +/** @typedef { import('@typescript-eslint/experimental-utils').TSESTree.ExportSpecifier} ExportSpecifier */ + +module.exports = createRule({ + name: 'ban-context-export', + defaultOptions: [], + meta: { + schema: [], + type: 'problem', + docs: { + description: 'Ban export of React context or context selector objects', + category: 'Best Practices', + recommended: 'error', + }, + messages: { + nativeContext: 'Trying to export React context{{exportName}} from {{filename}}', + contextSelector: 'Trying to export @fluentui/react-context-selector context {{exportName}} from {{filename}}', + }, + }, + create(context) { + 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 (!currentfileName.endsWith('src/index.ts')) { + 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); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.test.js b/packages/eslint-plugin/src/rules/ban-context-export/index.test.js new file mode 100644 index 0000000000000..7b72453a40bf8 --- /dev/null +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.test.js @@ -0,0 +1,77 @@ +// @ts-check +const { ESLintUtils } = require('@typescript-eslint/experimental-utils'); +const path = require('path'); +const rule = require('./index'); + +const ruleTester = new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + project: path.resolve(__dirname, './fixtures/ban-context-export/tsconfig.json'), + tsconfigRootDir: path.resolve(__dirname, './fixtures/ban-context-export'), + }, +}); + +/** + * @param {string} fixtureName + */ +function getParserOptions(fixtureName) { + return { + project: path.resolve(__dirname, `./fixtures/${fixtureName}/tsconfig.json`), + tsconfigRootDir: path.resolve(__dirname, `./fixtures/${fixtureName}`), + }; +} + +ruleTester.run('ban-context-export', rule, { + valid: [ + { + parserOptions: getParserOptions('internal-export'), + code: ` + export { MyContext } from './context' + `, + filename: 'src/internal/index.ts', + }, + { + parserOptions: getParserOptions('not-a-context'), + code: ` + export { MyContext } from './context' + `, + filename: 'src/index.ts', + }, + ], + invalid: [ + { + errors: [{ messageId: 'nativeContext' }], + parserOptions: getParserOptions('export-specifier'), + code: ` + export { MyContext } from './context' + `, + filename: 'src/index.ts', + }, + { + errors: [{ messageId: 'contextSelector' }], + parserOptions: getParserOptions('context-selector'), + code: ` + export { MyContext } from './context' + `, + filename: 'src/index.ts', + }, + { + errors: [{ messageId: 'nativeContext' }], + parserOptions: getParserOptions('named-export'), + code: ` + import * as React from 'react'; + export const MyContext = React.createContext({}); + `, + filename: 'src/index.ts', + }, + { + errors: [{ messageId: 'contextSelector' }], + parserOptions: getParserOptions('named-export'), + code: ` + import { createContext } from '@fluentui/react-context-selector'; + export const MyContext = createContext({}); + `, + filename: 'src/index.ts', + }, + ], +}); From 876d63f301a60ad357c86af0e89ecd92a30283ef Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 17:09:42 +0200 Subject: [PATCH 03/18] changefile --- ...eslint-plugin-bae0af9c-456e-420c-9560-08c80175e2de.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-eslint-plugin-bae0af9c-456e-420c-9560-08c80175e2de.json diff --git a/change/@fluentui-eslint-plugin-bae0af9c-456e-420c-9560-08c80175e2de.json b/change/@fluentui-eslint-plugin-bae0af9c-456e-420c-9560-08c80175e2de.json new file mode 100644 index 0000000000000..bfbe80251cd64 --- /dev/null +++ b/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" +} From b9d4df1f6c05caea8f61d8262101a81d55dc36fd Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 17:12:16 +0200 Subject: [PATCH 04/18] improve variable name --- packages/eslint-plugin/src/rules/ban-context-export/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js index b892358195861..657b648b94923 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -31,8 +31,8 @@ module.exports = createRule({ * @param {string} exportName */ function checkContextType(node, exportName) { - const currentfileName = context.getFilename(); - if (!currentfileName.endsWith('src/index.ts')) { + const isTopLevelExport = context.getFilename().endsWith('src/index.ts'); + if (!isTopLevelExport) { return; } From e7513a83e9c4456f4d63b384d4825051ba000db6 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 17:13:50 +0200 Subject: [PATCH 05/18] test on CI --- packages/eslint-plugin/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index a4d3ce3a14944..15fc3b5ca80a1 100644 --- a/packages/eslint-plugin/package.json +++ b/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", From c6ee2574e53ba43a7ee7b7514dfcbc1378407742 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 23 May 2022 17:14:29 +0200 Subject: [PATCH 06/18] fix type --- packages/eslint-plugin/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 15fc3b5ca80a1..523039b7c4541 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -10,7 +10,7 @@ "license": "MIT", "scripts": { "lint": "tsc --noEmit && eslint --ext .js --cache .", - "test": "yarn jest, --passWithNoTests" + "test": "yarn jest --passWithNoTests" }, "dependencies": { "@rnx-kit/eslint-plugin": "^0.2.5", From a956eeabeb5601f1e87b9ffedc234bd855b7373c Mon Sep 17 00:00:00 2001 From: ling1726 Date: Mon, 23 May 2022 18:10:25 +0200 Subject: [PATCH 07/18] Update packages/eslint-plugin/src/rules/ban-context-export/index.js Co-authored-by: Sean Monahan --- packages/eslint-plugin/src/rules/ban-context-export/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js index 657b648b94923..3a90d1bf42cad 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -17,7 +17,7 @@ module.exports = createRule({ recommended: 'error', }, messages: { - nativeContext: 'Trying to export React context{{exportName}} from {{filename}}', + nativeContext: 'Trying to export React context {{exportName}} from {{filename}}', contextSelector: 'Trying to export @fluentui/react-context-selector context {{exportName}} from {{filename}}', }, }, From 1961dc3f575569548311c655cc3b5da60b488b83 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 10:38:51 +0200 Subject: [PATCH 08/18] fix ts --- packages/eslint-plugin/src/rules/ban-context-export/index.js | 4 ++-- packages/eslint-plugin/tsconfig.json | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js index 657b648b94923..692a22d4a909f 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -80,12 +80,12 @@ module.exports = createRule({ // 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) } */ + /** @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) } */ + /** @type { import('@typescript-eslint/experimental-utils').TSESTree.Identifier } */ const identifier = declaration.id; identifierName = identifier.name; } diff --git a/packages/eslint-plugin/tsconfig.json b/packages/eslint-plugin/tsconfig.json index 880cdd2128c78..d824db5ff0f99 100644 --- a/packages/eslint-plugin/tsconfig.json +++ b/packages/eslint-plugin/tsconfig.json @@ -10,5 +10,6 @@ "strict": true, "types": ["node"] }, - "include": ["src/**/*"] + "include": ["src/**/*"], + "exclude": ["fixtures/**/*"] } From dd9fd2a5d94b43de29fcd6f70f3ff006dfd964f2 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 10:45:26 +0200 Subject: [PATCH 09/18] better message --- packages/eslint-plugin/src/rules/ban-context-export/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js index 5d7ff566d78c2..ac254e44c0331 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -17,8 +17,8 @@ module.exports = createRule({ recommended: 'error', }, messages: { - nativeContext: 'Trying to export React context {{exportName}} from {{filename}}', - contextSelector: 'Trying to export @fluentui/react-context-selector context {{exportName}} from {{filename}}', + nativeContext: '{{exportName}} should not be exported directly', + contextSelector: '{{exportName}} should not be exported directly', }, }, create(context) { From a91c537cda8b923f52168cf68120cf22bd3daa75 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 13:42:02 +0200 Subject: [PATCH 10/18] exclude fixtures --- packages/eslint-plugin/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tsconfig.json b/packages/eslint-plugin/tsconfig.json index d824db5ff0f99..1ad0e7809d02d 100644 --- a/packages/eslint-plugin/tsconfig.json +++ b/packages/eslint-plugin/tsconfig.json @@ -11,5 +11,5 @@ "types": ["node"] }, "include": ["src/**/*"], - "exclude": ["fixtures/**/*"] + "exclude": ["**/fixtures/**/*"] } From 886fd68d6a6758d6549c80ab25cc4efa2771934b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 20:50:54 +0200 Subject: [PATCH 11/18] add exclude option --- packages/eslint-plugin/package.json | 1 + packages/eslint-plugin/src/configs/core.js | 2 +- .../exclude/special-path/src/index.ts | 2 ++ .../fixtures/exclude/tsconfig.json | 11 ++++++ .../src/rules/ban-context-export/index.js | 34 +++++++++++++++++-- .../rules/ban-context-export/index.test.js | 23 +++++++++++++ 6 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/special-path/src/index.ts create mode 100644 packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/tsconfig.json diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 523039b7c4541..fc2e33270397e 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -27,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.2", "jju": "^1.4.0" }, "peerDependencies": { diff --git a/packages/eslint-plugin/src/configs/core.js b/packages/eslint-plugin/src/configs/core.js index cbd958cf7f02d..5a81a24132224 100644 --- a/packages/eslint-plugin/src/configs/core.js +++ b/packages/eslint-plugin/src/configs/core.js @@ -238,7 +238,7 @@ const typeAwareRules = { * plugin: https://github.com/gund/eslint-plugin-deprecation */ 'deprecation/deprecation': 'error', - '@fluentui/ban-context-export': 'error', + '@fluentui/ban-context-export': ['error', { exclude: ['**/react-shared-contexts/**'] }], }; /** diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/special-path/src/index.ts b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/special-path/src/index.ts new file mode 100644 index 0000000000000..f184fa5140be3 --- /dev/null +++ b/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 diff --git a/packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/tsconfig.json b/packages/eslint-plugin/src/rules/ban-context-export/fixtures/exclude/tsconfig.json new file mode 100644 index 0000000000000..0703633adb886 --- /dev/null +++ b/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"] + } +} diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.js b/packages/eslint-plugin/src/rules/ban-context-export/index.js index ac254e44c0331..ab7b61bf9ce1c 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.js @@ -1,15 +1,38 @@ // @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: [], + 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', @@ -22,6 +45,8 @@ module.exports = createRule({ }, }, 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; @@ -31,7 +56,12 @@ module.exports = createRule({ * @param {string} exportName */ function checkContextType(node, exportName) { - const isTopLevelExport = context.getFilename().endsWith('src/index.ts'); + const currentFileName = context.getFilename(); + if (exclude.some(pattern => minimatch(currentFileName, pattern))) { + return; + } + + const isTopLevelExport = currentFileName.endsWith('src/index.ts'); if (!isTopLevelExport) { return; } diff --git a/packages/eslint-plugin/src/rules/ban-context-export/index.test.js b/packages/eslint-plugin/src/rules/ban-context-export/index.test.js index 7b72453a40bf8..2088ed2c7f30b 100644 --- a/packages/eslint-plugin/src/rules/ban-context-export/index.test.js +++ b/packages/eslint-plugin/src/rules/ban-context-export/index.test.js @@ -37,6 +37,17 @@ ruleTester.run('ban-context-export', rule, { `, filename: 'src/index.ts', }, + { + // No way to type generics with jsdoc + // @ts-ignore + options: [{ exclude: ['**/special-path/**/*'] }], + parserOptions: getParserOptions('exclude'), + code: ` + import * as React from 'react'; + export const MyContext = React.createContext({}); + `, + filename: 'special-path/src/index.ts', + }, ], invalid: [ { @@ -73,5 +84,17 @@ ruleTester.run('ban-context-export', rule, { `, filename: 'src/index.ts', }, + { + errors: [{ messageId: 'nativeContext' }], + // No way to type generics with jsdoc + // @ts-ignore + options: [{ exclude: ['**/wrong-path/**/*'] }], + parserOptions: getParserOptions('exclude'), + code: ` + import * as React from 'react'; + export const MyContext = React.createContext({}); + `, + filename: 'special-path/src/index.ts', + }, ], }); From 073e6b115cd2e2afeb52d12f7c92626e726b450b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 21:14:52 +0200 Subject: [PATCH 12/18] fix syncpack --- packages/eslint-plugin/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index fc2e33270397e..b391ee35d1619 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -27,7 +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.2", + "minimatch": "^3.0.4", "jju": "^1.4.0" }, "peerDependencies": { From 7afdf23ce83a032d0240b79f160f5890db37fc06 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 21:16:24 +0200 Subject: [PATCH 13/18] add ignore --- packages/react-components/react-utilities/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-components/react-utilities/src/index.ts b/packages/react-components/react-utilities/src/index.ts index d6cde7a98e226..4749893eff58b 100644 --- a/packages/react-components/react-utilities/src/index.ts +++ b/packages/react-components/react-utilities/src/index.ts @@ -46,6 +46,7 @@ export type { UseOnClickOrScrollOutsideOptions, } from './hooks/index'; +// eslint-disable-next-line @fluentui/ban-context-export export { canUseDOM, defaultSSRContextValue, useIsSSR, useSSRContext, SSRContext, SSRProvider } from './ssr/index'; export type { SSRContextValue } from './ssr/index'; From bece35aa999c8f9d9a0b3f5f47671263e235ce95 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 21:28:30 +0200 Subject: [PATCH 14/18] changefile --- ...act-utilities-5fa1f06a-3abd-457a-b2b4-0dd403f861fd.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-utilities-5fa1f06a-3abd-457a-b2b4-0dd403f861fd.json diff --git a/change/@fluentui-react-utilities-5fa1f06a-3abd-457a-b2b4-0dd403f861fd.json b/change/@fluentui-react-utilities-5fa1f06a-3abd-457a-b2b4-0dd403f861fd.json new file mode 100644 index 0000000000000..c8e4b327fff8d --- /dev/null +++ b/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" +} From 5721861dc03bfe4975f2843ea3940dc5dea7c3e9 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 22:15:35 +0200 Subject: [PATCH 15/18] add eslint ignore --- packages/react-components/react-context-selector/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-components/react-context-selector/src/index.ts b/packages/react-components/react-context-selector/src/index.ts index 143a1c1951e62..bbc42ef78afce 100644 --- a/packages/react-components/react-context-selector/src/index.ts +++ b/packages/react-components/react-context-selector/src/index.ts @@ -1,4 +1,5 @@ export { createContext } from './createContext'; export { useContextSelector } from './useContextSelector'; export { useHasParentContext } from './useHasParentContext'; +// eslint-disable-next-line @fluentui/ban-context-export export type { Context, ContextSelector, ContextValue, ContextValues, ContextVersion } from './types'; From a5aea569cb71a03c43f9e03aa94502ea99460ff5 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 24 May 2022 22:16:26 +0200 Subject: [PATCH 16/18] changefile --- ...text-selector-e5b506ef-63ca-4690-8e76-eac9690a95f6.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-context-selector-e5b506ef-63ca-4690-8e76-eac9690a95f6.json diff --git a/change/@fluentui-react-context-selector-e5b506ef-63ca-4690-8e76-eac9690a95f6.json b/change/@fluentui-react-context-selector-e5b506ef-63ca-4690-8e76-eac9690a95f6.json new file mode 100644 index 0000000000000..5c4df3e235214 --- /dev/null +++ b/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" +} From 36a4b773b17907ddb3fd6a64359af22eb1e026df Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 25 May 2022 00:59:25 +0200 Subject: [PATCH 17/18] rule should only apply to v9 --- packages/eslint-plugin/src/configs/base.js | 1 + packages/eslint-plugin/src/configs/core.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/configs/base.js b/packages/eslint-plugin/src/configs/base.js index d5de005e17200..24e842c45c21f 100644 --- a/packages/eslint-plugin/src/configs/base.js +++ b/packages/eslint-plugin/src/configs/base.js @@ -20,6 +20,7 @@ module.exports = { rules: { // TODO: propagate to `error` once all packages barrel files have been fixed '@rnx-kit/no-export-all': ['warn', { expand: 'all' }], + '@fluentui/ban-context-export': ['error', { exclude: ['**/react-shared-contexts/**'] }], }, }, ], diff --git a/packages/eslint-plugin/src/configs/core.js b/packages/eslint-plugin/src/configs/core.js index 5a81a24132224..b6a69150c2b9b 100644 --- a/packages/eslint-plugin/src/configs/core.js +++ b/packages/eslint-plugin/src/configs/core.js @@ -238,7 +238,6 @@ const typeAwareRules = { * plugin: https://github.com/gund/eslint-plugin-deprecation */ 'deprecation/deprecation': 'error', - '@fluentui/ban-context-export': ['error', { exclude: ['**/react-shared-contexts/**'] }], }; /** From fdf2022f62be66002345181c495fdffec8b1ea45 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 25 May 2022 09:49:29 +0200 Subject: [PATCH 18/18] type aware rules for base --- packages/eslint-plugin/src/configs/base.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/configs/base.js b/packages/eslint-plugin/src/configs/base.js index 24e842c45c21f..54a1e8344d7f7 100644 --- a/packages/eslint-plugin/src/configs/base.js +++ b/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,12 +21,13 @@ module.exports = { ...getNamingConventionRule(), }, overrides: [ + // Enable rules requiring type info only for appropriate files/circumstances + ...configHelpers.getTypeInfoRuleOverrides(typeAwareRules), { files: '**/src/index.{ts,tsx,js}', rules: { // TODO: propagate to `error` once all packages barrel files have been fixed '@rnx-kit/no-export-all': ['warn', { expand: 'all' }], - '@fluentui/ban-context-export': ['error', { exclude: ['**/react-shared-contexts/**'] }], }, }, ],