diff --git a/change/@fluentui-api-docs-86aa1957-a259-47fd-8b0f-af884969e964.json b/change/@fluentui-api-docs-86aa1957-a259-47fd-8b0f-af884969e964.json new file mode 100644 index 00000000000000..c49a08fa2df7d5 --- /dev/null +++ b/change/@fluentui-api-docs-86aa1957-a259-47fd-8b0f-af884969e964.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "style: fix lint errors", + "packageName": "@fluentui/api-docs", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-codemods-b3d59667-a8d0-443e-ac85-72d2ea2e72a8.json b/change/@fluentui-codemods-b3d59667-a8d0-443e-ac85-72d2ea2e72a8.json new file mode 100644 index 00000000000000..0c8c34eb2ccec1 --- /dev/null +++ b/change/@fluentui-codemods-b3d59667-a8d0-443e-ac85-72d2ea2e72a8.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "style: fix lint errors", + "packageName": "@fluentui/codemods", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-eslint-plugin-54a8dcfd-724c-4111-a35e-1320a6dd6cae.json b/change/@fluentui-eslint-plugin-54a8dcfd-724c-4111-a35e-1320a6dd6cae.json new file mode 100644 index 00000000000000..c0fb69e56368a7 --- /dev/null +++ b/change/@fluentui-eslint-plugin-54a8dcfd-724c-4111-a35e-1320a6dd6cae.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "feat: create core base for react/node/legacy lint configs", + "packageName": "@fluentui/eslint-plugin", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/packages/api-docs/src/generatePageJsonFiles.ts b/packages/api-docs/src/generatePageJsonFiles.ts index b2993750b050b8..fb1c2a91625de8 100644 --- a/packages/api-docs/src/generatePageJsonFiles.ts +++ b/packages/api-docs/src/generatePageJsonFiles.ts @@ -25,6 +25,7 @@ export function generatePageJsonFiles(options: IPageJsonOptions): void { // Load api-extractor output from packages into a model const apiModel = new ApiModel(); for (const apiJsonPath of apiJsonPaths) { + // eslint-disable-next-line no-console console.log('Loading ' + apiJsonPath); // If the file belongs to the compat layer. @@ -53,6 +54,7 @@ export function generatePageJsonFiles(options: IPageJsonOptions): void { const requestedPages = ([] as string[]).concat(...Object.values(pageGroups)); for (const pageName of requestedPages) { if (!pageJsonByName.has(pageName)) { + // eslint-disable-next-line no-console console.warn('Warning: no API items found for expected @docCategory ' + pageName); } } @@ -61,6 +63,7 @@ export function generatePageJsonFiles(options: IPageJsonOptions): void { for (const [pageName, pageJson] of pageJsonByName.entries()) { const pageJsonPath = path.join(outputRoot, pageJson.group || '', pageName + '.page.json'); + // eslint-disable-next-line no-console console.log('Writing ' + pageJsonPath); const json = min ? JSON.stringify(pageJson) : JSON.stringify(pageJson, null, 2); fse.writeFileSync(pageJsonPath, json); diff --git a/packages/codemods/.eslintrc.json b/packages/codemods/.eslintrc.json index 05f2e3df0405e5..3659e1a174fbb0 100644 --- a/packages/codemods/.eslintrc.json +++ b/packages/codemods/.eslintrc.json @@ -1,5 +1,5 @@ { - "extends": ["plugin:@fluentui/eslint-plugin/node"], + "extends": ["plugin:@fluentui/eslint-plugin/node", "plugin:@fluentui/eslint-plugin/react"], "root": true, "overrides": [ { diff --git a/packages/codemods/src/command.ts b/packages/codemods/src/command.ts index a93b37bc2f3b57..d8bb4c3f9ac505 100644 --- a/packages/codemods/src/command.ts +++ b/packages/codemods/src/command.ts @@ -73,8 +73,10 @@ export class CommandParser { } if (parsed.list) { const mods = getEnabledMods(console, getModsPaths); + // eslint-disable-next-line no-console console.log('Here are the enabled code mod names:\n'); mods.forEach(mod => { + // eslint-disable-next-line no-console console.log(mod.name); }); return { shouldExit: true, modsFilter: () => true }; @@ -86,6 +88,7 @@ export class CommandParser { if (configResult.ok) { configObj = configResult.value; } else { + // eslint-disable-next-line no-console console.log(configResult.value); return { shouldExit: true, modsFilter: () => true }; } @@ -115,6 +118,7 @@ function getModRunnerConfig(): Result { sync: true, }); let configObj: ModRunnerConfigType = { stringFilters: [], regexFilters: [], includeMods: false }; + // eslint-disable-next-line no-console console.log('Configuration detected. Attempting to run mods from config...'); if (!foundJsonFile.found || foundJsonFile.found.length !== 1) { return Err({ error: new Error('Error, could not locate correct config file.') }); diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 4feecd7fe52645..71aac12a6f9d5e 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -6,11 +6,12 @@ Usage: in your [ESLint config file](https://eslint.org/docs/user-guide/configuring), add `{ "extends": ["plugin:@fluentui/"] }` or `{ "extends": ["plugin:@fluentui/eslint-plugin/"] }` (the two are equivalent). -- `react`: For `@fluentui/react` and related packages - - `react--legacy`: Like `react` but requiring an `I` prefix for interfaces - - `node`: Like `react` but for packages which run in a Node environment (not the browser) - - `node--legacy`: Like `node` but requiring an `I` prefix for interfaces +- `react`: react specific configuration for fluentui vNext +- `node`: node specific configuration for fluentui vNext +- `react--legacy`: react specific configuration for fluentui v7,8 +- `node--legacy`: node specific configuration for fluentui v7,8 - `react-northstar`: For `@fluentui/react-northstar` and related packages +- `imports`: auto import statements sorting configuration Helpers for customizing configuration are exported under a `configHelpers` object. diff --git a/packages/eslint-plugin/src/configs/base-legacy.js b/packages/eslint-plugin/src/configs/base-legacy.js new file mode 100644 index 00000000000000..24b6140a3eb2ff --- /dev/null +++ b/packages/eslint-plugin/src/configs/base-legacy.js @@ -0,0 +1,18 @@ +// @ts-check + +const path = require('path'); + +const { getNamingConventionRule } = require('../utils/configHelpers'); + +/** @type {import("eslint").Linter.Config} */ +module.exports = { + extends: [path.join(__dirname, 'core')], + rules: { + /** + * `@typescript-eslint`plugin eslint rules + * @see https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/eslint-plugin + */ + ...getNamingConventionRule({ prefixInterface: true }), + }, + overrides: [], +}; diff --git a/packages/eslint-plugin/src/configs/base.js b/packages/eslint-plugin/src/configs/base.js new file mode 100644 index 00000000000000..d5de005e172002 --- /dev/null +++ b/packages/eslint-plugin/src/configs/base.js @@ -0,0 +1,26 @@ +// @ts-check + +const path = require('path'); + +const { getNamingConventionRule } = require('../utils/configHelpers'); + +/** @type {import("eslint").Linter.Config} */ +module.exports = { + extends: [path.join(__dirname, 'core')], + rules: { + /** + * `@typescript-eslint`plugin eslint rules + * @see https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/eslint-plugin + */ + ...getNamingConventionRule(), + }, + overrides: [ + { + 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' }], + }, + }, + ], +}; diff --git a/packages/eslint-plugin/src/configs/core.js b/packages/eslint-plugin/src/configs/core.js new file mode 100644 index 00000000000000..b6a69150c2b9b3 --- /dev/null +++ b/packages/eslint-plugin/src/configs/core.js @@ -0,0 +1,357 @@ +// @ts-check +const configHelpers = require('../utils/configHelpers'); + +/** @type {import("eslint").Linter.Config} */ +const config = { + root: true, + extends: [ + // Provides both rules and some parser options and other settings + 'airbnb/base', + // add typescript support for import plugin - https://github.com/import-js/eslint-plugin-import/blob/main/config/typescript.js + 'plugin:import/typescript', + // Extended configs are applied in order, so these configs that turn other rules off should come last + 'prettier', + ], + parser: '@typescript-eslint/parser', + plugins: ['import', '@fluentui', '@rnx-kit', '@typescript-eslint', 'deprecation', 'jest', 'jsdoc'], + settings: { + 'import/resolver': { + // @see https://github.com/alexgorbatchev/eslint-import-resolver-typescript#configuration + typescript: { + alwaysTryTypes: true, + project: './tsconfig.json', + }, + }, + jsdoc: { + ignoreInternal: true, + tagNamePreference: { + // Allow any of @default, @defaultvalue, @defaultValue until we settle on a preferred one + default: 'default', + defaultvalue: 'defaultvalue', + defaultValue: 'defaultValue', + // Allow either @return or @returns until we settle on a preferred one + return: 'return', + returns: 'returns', + }, + }, + }, + env: { + browser: true, + 'jest/globals': true, + }, + // We have to disable this when running lint-staged, or it will incorrectly flag eslint-disable + // directives for rules which are disabled only in that context. + reportUnusedDisableDirectives: !configHelpers.isLintStaged, + // matched relative to cwd + ignorePatterns: [ + 'coverage', + 'dist', + 'dist-storybook', + 'etc', + 'lib', + 'lib-amd', + 'lib-commonjs', + 'node_modules', + 'temp', + '**/__snapshots__', + '**/*.scss.ts', + ], + rules: { + /** + * core eslint rules + * @see https://eslint.org/docs/rules + */ + curly: ['error', 'all'], + 'dot-notation': 'error', + eqeqeq: ['error', 'always'], + 'guard-for-in': 'error', + 'no-alert': 'error', + 'no-bitwise': 'error', + 'no-caller': 'error', + 'no-console': 'error', + 'no-constant-condition': 'error', + 'no-debugger': 'error', + 'no-duplicate-case': 'error', + 'no-empty': 'error', + 'no-eval': 'error', + 'no-new-wrappers': 'error', + 'no-restricted-globals': [ + 'error', + ...['blur', 'close', 'focus', 'length', 'name', 'parent', 'self', 'stop'].map(name => ({ + name, + message: `"${name}" refers to a DOM global. Did you mean to reference a local value instead?`, + })), + ], + 'no-restricted-properties': [ + 'error', + { object: 'describe', property: 'only', message: 'describe.only should only be used during test development' }, + { object: 'it', property: 'only', message: 'it.only should only be used during test development' }, + { + object: 'React', + property: 'useLayoutEffect', + message: '`useLayoutEffect` causes a warning in SSR. Use `useIsomorphicLayoutEffect`', + }, + ], + 'no-shadow': ['error', { hoist: 'all' }], + 'no-var': 'error', + 'prefer-const': 'error', + 'prefer-arrow-callback': 'error', // tslint: no-function-expression + radix: ['error', 'always'], + 'lines-between-class-members': 'off', + 'max-classes-per-file': 'off', + 'no-case-declarations': 'off', + 'no-cond-assign': 'off', + 'no-continue': 'off', + 'no-control-regex': 'off', + 'no-else-return': 'off', + 'no-lonely-if': 'off', + 'no-loop-func': 'off', + 'no-multi-assign': 'off', + 'no-nested-ternary': 'off', + 'no-param-reassign': 'off', + 'no-plusplus': 'off', + 'no-prototype-builtins': 'off', + 'no-return-assign': 'off', + 'no-template-curly-in-string': 'off', + 'no-undef-init': 'off', + 'no-underscore-dangle': 'off', + 'no-unneeded-ternary': 'off', + 'no-unused-expressions': 'off', + 'no-use-before-define': 'off', + 'no-useless-computed-key': 'off', + 'no-useless-concat': 'off', + 'no-useless-constructor': 'off', + 'no-useless-escape': 'off', + 'no-useless-rename': 'off', + 'no-useless-return': 'off', + 'object-shorthand': 'off', + 'operator-assignment': 'off', + 'prefer-destructuring': 'off', + 'prefer-template': 'off', + // airbnb or other config overrides (some temporary) + // TODO: determine which rules we want to enable, and make needed changes (separate PR) + 'arrow-body-style': 'off', + 'class-methods-use-this': 'off', + 'consistent-return': 'off', + 'default-case': 'off', + 'func-names': 'off', + 'global-require': 'off', + 'spaced-comment': 'off', + // airbnb options ban for-of which is unnecessary for TS and modern node (https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/style.js#L334) + // but this is a very powerful rule we may want to use in other ways + 'no-restricted-syntax': 'off', + // permanently disable because we disagree with these rules + 'no-await-in-loop': 'off', // contrary to rule docs, awaited things often are NOT parallelizable + // permanently disable due to performance issues (using custom rule `@fluentui/max-len` instead) + 'max-len': 'off', + // permanently disable due to perf problems and limited benefit + // see here for perf testing (note that you must run eslint directly) + // https://eslint.org/docs/developer-guide/working-with-rules#per-rule-performance + 'no-empty-character-class': 'off', + + /** + * `@typescript-eslint`plugin eslint rules + * @see https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/eslint-plugin + */ + '@typescript-eslint/no-empty-function': 'error', + '@typescript-eslint/no-explicit-any': 'error', + '@typescript-eslint/prefer-namespace-keyword': 'error', + + /** + * `@rnx-kit` eslint rules + * @see https://github.com/microsoft/rnx-kit/tree/main/packages/eslint-plugin + */ + '@rnx-kit/no-export-all': ['error', { expand: 'external-only' }], + + /** + * `@fluentui` eslint rules / This package + * @see https://www.npmjs.com/package/@fluentui/eslint-plugin + */ + '@fluentui/ban-imports': [ + 'error', + { + path: 'react', + names: ['useLayoutEffect'], + message: '`useLayoutEffect` causes a warning in SSR. Use `useIsomorphicLayoutEffect`', + }, + ], + '@fluentui/no-global-react': 'error', + '@fluentui/max-len': [ + 'error', + { + ignorePatterns: [ + 'require(<.*?>)?\\(', + 'https?:\\/\\/', + '^(import|export) ', + '^\\s+( [ + // Enable rules requiring type info only for appropriate files/circumstances + ...configHelpers.getTypeInfoRuleOverrides(typeAwareRules), + { + files: '**/*.{ts,tsx}', + // This turns off a few rules that don't work or are unnecessary for TS, and enables a few + // that make sense for TS: no-var, prefer-const, prefer-rest-params, prefer-spread + // https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/eslint-recommended.ts + extends: ['plugin:@typescript-eslint/eslint-recommended'], + // and manually enable rules that only work on TS + rules: { + '@typescript-eslint/ban-ts-comment': 'error', + '@typescript-eslint/explicit-member-accessibility': [ + 'error', + { + accessibility: 'explicit', + overrides: { constructors: 'off' }, + }, + ], + '@typescript-eslint/member-ordering': [ + 'error', + { + default: [ + 'public-static-field', + 'protected-static-field', + 'private-static-field', + 'public-instance-field', + 'protected-instance-field', + 'private-instance-field', + 'public-static-method', + 'protected-static-method', + 'private-static-method', + 'public-constructor', + 'public-instance-method', + 'protected-constructor', + 'protected-instance-method', + 'private-constructor', + 'private-instance-method', + ], + }, + ], + '@typescript-eslint/no-shadow': 'error', + + // permanently disable due to using other rules which do the same thing + camelcase: 'off', // redundant with @typescript-eslint/naming-convention + + // permanently disable due to improper TS handling or unnecessary for TS + // (and not covered by plugin:@typescript-eslint/eslint-recommended) + 'no-empty-function': 'off', + 'no-shadow': 'off', + 'no-unused-vars': 'off', + }, + }, + { + // Test overrides + files: [...configHelpers.testFiles], + rules: { + 'no-console': 'off', + }, + }, + { + files: 'src/**/*.deprecated.test.{ts,tsx}', + rules: { + 'deprecation/deprecation': 'off', // the purpose of these tests + }, + }, + { + // Example overrides + files: '**/*.{Example,stories}.tsx', + rules: { + 'no-alert': 'off', + 'no-console': 'off', + }, + }, + { + // Docs overrides (excluding examples) + files: [...configHelpers.docsFiles], + rules: { + 'import/no-webpack-loader-syntax': 'off', // this is ok in docs + }, + }, + { + files: [...configHelpers.configFiles], + rules: { + 'no-console': 'off', + }, + }, + { + files: [...configHelpers.devDependenciesFiles], + rules: { + // TODO: https://github.com/microsoft/fluentui/issues/21999 + 'import/no-extraneous-dependencies': 'off', + }, + }, +]; + +// Why use `defineProperty` for `overrides`? +// +// By default, any logic in this file will be run every time the plugin is loaded (even if this +// config is not used) due to it being included by necessity in the package index file. +// These overrides include some more complex logic which should only run when requested, since it's +// more costly and can cause build errors if run in a package it wasn't designed for. +// If ESLint supported exporting a function from a config file, that would be an easy solution. +// Since that's not supported, we work around it by defining overrides as a property with getter. +// @ts-ignore -- `overrides?` is declared in `eslint.Linter.Config` but our `config` object doesn't define it until now +Object.defineProperty(config, 'overrides', { + enumerable: true, + get: getOverrides, +}); + +module.exports = config; diff --git a/packages/eslint-plugin/src/configs/imports.js b/packages/eslint-plugin/src/configs/imports.js new file mode 100644 index 00000000000000..78bf2faedba87d --- /dev/null +++ b/packages/eslint-plugin/src/configs/imports.js @@ -0,0 +1,27 @@ +module.exports = { + rules: { + /** + * core eslint rules + * @see https://eslint.org/docs/rules + */ + // handles only member sort, rest is handled via import/order + 'sort-imports': ['warn', { ignoreDeclarationSort: true }], + + /** + * import plugin rules + * @see https://github.com/import-js/eslint-plugin-import + */ + 'import/no-duplicates': 'warn', + 'import/first': 'warn', + 'import/order': [ + 'warn', + { + 'newlines-between': 'always', + alphabetize: { + order: 'asc', + caseInsensitive: false, + }, + }, + ], + }, +}; diff --git a/packages/eslint-plugin/src/configs/node-legacy.js b/packages/eslint-plugin/src/configs/node-legacy.js index 5636a1c802bc44..8e36520c72c332 100644 --- a/packages/eslint-plugin/src/configs/node-legacy.js +++ b/packages/eslint-plugin/src/configs/node-legacy.js @@ -1,12 +1,9 @@ // @ts-check const path = require('path'); -const configHelpers = require('../utils/configHelpers'); /** @type {import("eslint").Linter.Config} */ module.exports = { - extends: [path.join(__dirname, 'node')], - rules: { - ...configHelpers.getNamingConventionRule(true /* prefixWithI */), - }, + extends: [path.join(__dirname, 'base-legacy')], + rules: {}, }; diff --git a/packages/eslint-plugin/src/configs/node.js b/packages/eslint-plugin/src/configs/node.js index c321742fd5f47c..91146bd8764dd4 100644 --- a/packages/eslint-plugin/src/configs/node.js +++ b/packages/eslint-plugin/src/configs/node.js @@ -4,7 +4,7 @@ const path = require('path'); /** @type {import("eslint").Linter.Config} */ module.exports = { - extends: [path.join(__dirname, 'react')], + extends: [path.join(__dirname, 'base')], rules: { 'no-console': 'off', }, diff --git a/packages/eslint-plugin/src/configs/react-config.js b/packages/eslint-plugin/src/configs/react-config.js new file mode 100644 index 00000000000000..e3a86533ddc77c --- /dev/null +++ b/packages/eslint-plugin/src/configs/react-config.js @@ -0,0 +1,136 @@ +const configHelpers = require('../utils/configHelpers'); + +/** @type {import("eslint").Linter.Config} */ +module.exports = { + plugins: ['jsx-a11y', 'react', 'react-hooks', '@griffel'], + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + settings: { + react: { + pragma: 'React', + version: 'detect', + }, + // copied from https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/rules/react.js#L575 + propWrapperFunctions: [ + 'forbidExtraProps', // https://www.npmjs.com/package/airbnb-prop-types + 'exact', // https://www.npmjs.com/package/prop-types-exact + 'Object.freeze', // https://tc39.github.io/ecma262/#sec-object.freeze + ], + }, + rules: { + /** + * griffel eslint rules + * @see https://github.com/microsoft/griffel/tree/main/packages/eslint-plugin + */ + '@griffel/no-shorthands': 'error', + /** + * react eslint rules + * @see https://github.com/yannickcr/eslint-plugin-react + */ + 'react/jsx-key': 'error', + 'react/jsx-no-bind': [ + 'error', + { + allowArrowFunctions: false, // tslint: jsx-no-lambda + allowFunctions: false, + allowBind: false, + ignoreDOMComponents: true, + ignoreRefs: true, + }, + ], + 'react/no-string-refs': 'error', + 'react/self-closing-comp': 'error', + 'react/no-danger': 'warn', + // NOTE: following rules has been turned off to override `airbnb/rules/react`, which we don't use anymore + // TODO: needs to be checked against core react eslint plugin if we need those turn off explicitly + // @see https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/rules/react.js + 'react/button-has-type': 'off', + 'react/destructuring-assignment': 'off', + 'react/forbid-prop-types': 'off', + 'react/jsx-boolean-value': 'off', + 'react/jsx-curly-brace-presence': 'off', + 'react/jsx-no-target-blank': 'off', + 'react/jsx-pascal-case': 'off', // Doesn't handle lowercase slot names + 'react/no-access-state-in-setstate': 'off', + 'react/no-array-index-key': 'off', + 'react/no-did-update-set-state': 'off', + 'react/no-find-dom-node': 'off', + 'react/no-render-return-value': 'off', + 'react/no-unescaped-entities': 'off', + 'react/no-will-update-set-state': 'off', + 'react/prefer-stateless-function': 'off', + 'react/sort-comp': 'off', + 'react/state-in-constructor': 'off', + 'react/static-property-placement': 'off', + 'react/require-default-props': 'off', + 'react/no-unknown-property': 'off', // expensive, limited benefit with TS + // these ones have minor negative perf impact and are unnecessary + 'react/default-props-match-prop-types': 'off', + 'react/no-unused-prop-types': 'off', + 'react/prefer-es6-class': 'off', + // permanently disable because we disagree with these rules + 'react/jsx-props-no-spreading': 'off', + 'react/prop-types': 'off', + 'react/jsx-filename-extension': 'off', + + /** + * react-hooks rules + * @see https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks + */ + 'react-hooks/exhaustive-deps': [ + 'error', + { + additionalHooks: 'useIsomorphicLayoutEffect', + }, + ], + 'react-hooks/rules-of-hooks': 'error', + + /** + * jsx-a11y rules + * @see https://github.com/jsx-eslint/eslint-plugin-jsx-a11y + */ + 'jsx-a11y/tabindex-no-positive': 'error', + // NOTE: following rules has been turned off to override `airbnb/rules/react-a11y` + // TODO: needs to be checked against core jsx-a11y eslint plugin if we need those turn off explicitly + // @see https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/rules/react-a11y.js + 'jsx-a11y/alt-text': 'off', + 'jsx-a11y/anchor-is-valid': 'off', + 'jsx-a11y/aria-activedescendant-has-tabindex': 'off', + 'jsx-a11y/aria-role': 'off', + 'jsx-a11y/click-events-have-key-events': 'off', + 'jsx-a11y/control-has-associated-label': 'off', + 'jsx-a11y/role-has-required-aria-props': 'off', + 'jsx-a11y/interactive-supports-focus': 'off', + 'jsx-a11y/label-has-associated-control': 'off', + 'jsx-a11y/media-has-caption': 'off', + 'jsx-a11y/mouse-events-have-key-events': 'off', + 'jsx-a11y/no-interactive-element-to-noninteractive-role': 'off', + 'jsx-a11y/no-noninteractive-element-interactions': 'off', + 'jsx-a11y/no-noninteractive-tabindex': 'off', + 'jsx-a11y/no-redundant-roles': 'off', + 'jsx-a11y/no-static-element-interactions': 'off', + 'jsx-a11y/role-supports-aria-props': 'off', + }, + overrides: [ + { + // Test overrides + files: [...configHelpers.testFiles], + rules: { + 'react/jsx-no-bind': 'off', + }, + }, + { + files: '**/*.stories.tsx', + rules: { + // allow arrow functions in stories for now (may want to change this later since using + // constantly-mutating functions can be an anti-pattern which we may not want to demonstrate + // in our converged components docs; it happened to be allowed starting out because .stories + // files were being linted as tests) + 'react/jsx-no-bind': 'off', + }, + }, + ], +}; diff --git a/packages/eslint-plugin/src/configs/react-legacy.js b/packages/eslint-plugin/src/configs/react-legacy.js index 119f72aca8418f..949646121d3692 100644 --- a/packages/eslint-plugin/src/configs/react-legacy.js +++ b/packages/eslint-plugin/src/configs/react-legacy.js @@ -1,14 +1,12 @@ // @ts-check const path = require('path'); -const configHelpers = require('../utils/configHelpers'); /** @type {import("eslint").Linter.Config} */ module.exports = { - extends: [path.join(__dirname, 'react')], + extends: [path.join(__dirname, 'base-legacy'), path.join(__dirname, 'react-config')], rules: { - ...configHelpers.getNamingConventionRule(true /* prefixWithI */), 'jsdoc/check-tag-names': 'off', '@griffel/no-shorthands': 'off', }, diff --git a/packages/eslint-plugin/src/configs/react.js b/packages/eslint-plugin/src/configs/react.js index e288804543435a..a50553f8390020 100644 --- a/packages/eslint-plugin/src/configs/react.js +++ b/packages/eslint-plugin/src/configs/react.js @@ -1,442 +1,9 @@ // @ts-check -const configHelpers = require('../utils/configHelpers'); -/** @type {import("eslint").Linter.Config} */ -const config = { - root: true, - extends: [ - // Provides both rules and some parser options and other settings - 'airbnb', - 'plugin:@griffel/recommended', - // add typescript support for import plugin - https://github.com/import-js/eslint-plugin-import/blob/main/config/typescript.js - 'plugin:import/typescript', - // Extended configs are applied in order, so these configs that turn other rules off should come last - 'prettier', - ], - parser: '@typescript-eslint/parser', - plugins: [ - 'import', - '@fluentui', - '@griffel', - '@rnx-kit', - '@typescript-eslint', - 'deprecation', - 'jest', - 'jsdoc', - 'jsx-a11y', - 'react', - 'react-hooks', - ], - settings: { - 'import/resolver': { - // @see https://github.com/alexgorbatchev/eslint-import-resolver-typescript#configuration - typescript: { - alwaysTryTypes: true, - project: './tsconfig.json', - }, - }, - jsdoc: { - ignoreInternal: true, - tagNamePreference: { - // Allow any of @default, @defaultvalue, @defaultValue until we settle on a preferred one - default: 'default', - defaultvalue: 'defaultvalue', - defaultValue: 'defaultValue', - // Allow either @return or @returns until we settle on a preferred one - return: 'return', - returns: 'returns', - }, - }, - }, - env: { - browser: true, - 'jest/globals': true, - }, - // We have to disable this when running lint-staged, or it will incorrectly flag eslint-disable - // directives for rules which are disabled only in that context. - reportUnusedDisableDirectives: !configHelpers.isLintStaged, - // matched relative to cwd - ignorePatterns: [ - 'coverage', - 'dist', - 'dist-storybook', - 'etc', - 'lib', - 'lib-amd', - 'lib-commonjs', - 'node_modules', - 'temp', - '**/__snapshots__', - '**/*.scss.ts', - ], - rules: { - '@rnx-kit/no-export-all': ['error', { expand: 'external-only' }], - '@fluentui/no-global-react': 'error', - '@fluentui/max-len': [ - 'error', - { - ignorePatterns: [ - 'require(<.*?>)?\\(', - 'https?:\\/\\/', - '^(import|export) ', - '^\\s+( ({ - name, - message: `"${name}" refers to a DOM global. Did you mean to reference a local value instead?`, - })), - ], - '@fluentui/ban-imports': [ - 'error', - { - path: 'react', - names: ['useLayoutEffect'], - message: '`useLayoutEffect` causes a warning in SSR. Use `useIsomorphicLayoutEffect`', - }, - ], - 'no-restricted-properties': [ - 'error', - { object: 'describe', property: 'only', message: 'describe.only should only be used during test development' }, - { object: 'it', property: 'only', message: 'it.only should only be used during test development' }, - { - object: 'React', - property: 'useLayoutEffect', - message: '`useLayoutEffect` causes a warning in SSR. Use `useIsomorphicLayoutEffect`', - }, - ], - 'no-shadow': ['error', { hoist: 'all' }], - 'no-var': 'error', - 'prefer-const': 'error', - 'prefer-arrow-callback': 'error', // tslint: no-function-expression - radix: ['error', 'always'], - 'react/jsx-key': 'error', - 'react/jsx-no-bind': [ - 'error', - { - allowArrowFunctions: false, // tslint: jsx-no-lambda - allowFunctions: false, - allowBind: false, - ignoreDOMComponents: true, - ignoreRefs: true, - }, - ], - 'react/no-string-refs': 'error', - 'react/self-closing-comp': 'error', - 'react-hooks/exhaustive-deps': [ - 'error', - { - additionalHooks: 'useIsomorphicLayoutEffect', - }, - ], - 'react-hooks/rules-of-hooks': 'error', - - // airbnb or other config overrides (some temporary) - // TODO: determine which rules we want to enable, and make needed changes (separate PR) - 'arrow-body-style': 'off', - 'class-methods-use-this': 'off', - 'consistent-return': 'off', - 'default-case': 'off', - 'func-names': 'off', - 'global-require': 'off', - - 'jsx-a11y/alt-text': 'off', - 'jsx-a11y/anchor-is-valid': 'off', - 'jsx-a11y/aria-activedescendant-has-tabindex': 'off', - 'jsx-a11y/aria-role': 'off', - 'jsx-a11y/click-events-have-key-events': 'off', - 'jsx-a11y/control-has-associated-label': 'off', - 'jsx-a11y/role-has-required-aria-props': 'off', - 'jsx-a11y/interactive-supports-focus': 'off', - 'jsx-a11y/label-has-associated-control': 'off', - 'jsx-a11y/media-has-caption': 'off', - 'jsx-a11y/mouse-events-have-key-events': 'off', - 'jsx-a11y/no-interactive-element-to-noninteractive-role': 'off', - 'jsx-a11y/no-noninteractive-element-interactions': 'off', - 'jsx-a11y/no-noninteractive-tabindex': 'off', - 'jsx-a11y/no-redundant-roles': 'off', - 'jsx-a11y/no-static-element-interactions': 'off', - 'jsx-a11y/role-supports-aria-props': 'off', - 'lines-between-class-members': 'off', - 'max-classes-per-file': 'off', - 'no-case-declarations': 'off', - 'no-cond-assign': 'off', - 'no-continue': 'off', - 'no-control-regex': 'off', - 'no-else-return': 'off', - 'no-lonely-if': 'off', - 'no-loop-func': 'off', - 'no-multi-assign': 'off', - 'no-nested-ternary': 'off', - 'no-param-reassign': 'off', - 'no-plusplus': 'off', - 'no-prototype-builtins': 'off', - 'no-return-assign': 'off', - 'no-template-curly-in-string': 'off', - 'no-undef-init': 'off', - 'no-underscore-dangle': 'off', - 'no-unneeded-ternary': 'off', - 'no-unused-expressions': 'off', - 'no-use-before-define': 'off', - 'no-useless-computed-key': 'off', - 'no-useless-concat': 'off', - 'no-useless-constructor': 'off', - 'no-useless-escape': 'off', - 'no-useless-rename': 'off', - 'no-useless-return': 'off', - 'object-shorthand': 'off', - 'operator-assignment': 'off', - 'prefer-destructuring': 'off', - 'prefer-template': 'off', - 'react/button-has-type': 'off', - 'react/destructuring-assignment': 'off', - 'react/forbid-prop-types': 'off', - 'react/jsx-boolean-value': 'off', - 'react/jsx-curly-brace-presence': 'off', - 'react/jsx-no-target-blank': 'off', - 'react/jsx-pascal-case': 'off', // Doesn't handle lowercase slot names - 'react/no-access-state-in-setstate': 'off', - 'react/no-array-index-key': 'off', - 'react/no-did-update-set-state': 'off', - 'react/no-find-dom-node': 'off', - 'react/no-render-return-value': 'off', - 'react/no-unescaped-entities': 'off', - 'react/no-will-update-set-state': 'off', - 'react/prefer-stateless-function': 'off', - 'react/sort-comp': 'off', - 'react/state-in-constructor': 'off', - 'react/static-property-placement': 'off', - 'react/require-default-props': 'off', - 'spaced-comment': 'off', - - // airbnb options ban for-of which is unnecessary for TS and modern node (https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/rules/style.js#L334) - // but this is a very powerful rule we may want to use in other ways - 'no-restricted-syntax': 'off', - - // permanently disable because we disagree with these rules - 'no-await-in-loop': 'off', // contrary to rule docs, awaited things often are NOT parallelizable - 'react/jsx-props-no-spreading': 'off', - 'react/prop-types': 'off', - - // permanently disable due to performance issues (using custom rule `@fluentui/max-len` instead) - 'max-len': 'off', - - // permanently disable due to perf problems and limited benefit - // see here for perf testing (note that you must run eslint directly) - // https://eslint.org/docs/developer-guide/working-with-rules#per-rule-performance - 'no-empty-character-class': 'off', - 'react/no-unknown-property': 'off', // expensive, limited benefit with TS - // these ones have minor negative perf impact and are unnecessary - 'react/default-props-match-prop-types': 'off', - 'react/no-unused-prop-types': 'off', - 'react/prefer-es6-class': 'off', +const path = require('path'); - 'jsdoc/check-tag-names': [ - 'error', - { - // Allow TSDoc tags @remarks and @defaultValue - definedTags: ['remarks', 'defaultValue'], - }, - ], - - /** - * - * import plugin rules - * @see https://github.com/import-js/eslint-plugin-import - */ - 'import/no-extraneous-dependencies': ['error', { devDependencies: false }], - 'import/extensions': 'off', - 'import/first': 'off', - 'import/newline-after-import': 'off', - 'import/no-duplicates': 'off', // mostly redundant with no-duplicate-imports - 'import/no-dynamic-require': 'off', - 'import/no-mutable-exports': 'off', - 'import/no-unresolved': 'off', - 'import/no-useless-path-segments': 'off', - 'import/order': 'off', - 'import/prefer-default-export': 'off', - // may cause perf problems per https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#eslint-plugin-import - 'import/no-cycle': 'off', - 'import/no-deprecated': 'off', - 'import/no-named-as-default': 'off', - 'import/no-unused-modules': 'off', - // these ones aren't needed for TS and may cause perf problems - 'import/default': 'off', - 'import/namespace': 'off', - 'import/no-named-as-default-member': 'off', - 'import/export': 'off', - }, -}; - -/** @type {import("eslint").Linter.RulesRecord} */ -const typeAwareRules = { - /** - * plugin: https://github.com/gund/eslint-plugin-deprecation - */ - 'deprecation/deprecation': 'error', +/** @type {import("eslint").Linter.Config} */ +module.exports = { + extends: [path.join(__dirname, 'base'), path.join(__dirname, 'react-config')], + rules: {}, }; - -/** - * Override definitions for `config`. See explanation at bottom of file for why/how this function is used. - * @returns {import("eslint").Linter.ConfigOverride[]} - */ -const getOverrides = () => [ - // 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' }], - }, - }, - { - files: '**/*.{ts,tsx}', - // This turns off a few rules that don't work or are unnecessary for TS, and enables a few - // that make sense for TS: no-var, prefer-const, prefer-rest-params, prefer-spread - // https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/eslint-recommended.ts - extends: ['plugin:@typescript-eslint/eslint-recommended'], - // and manually enable rules that only work on TS - rules: { - '@typescript-eslint/ban-ts-comment': 'error', - '@typescript-eslint/explicit-member-accessibility': [ - 'error', - { - accessibility: 'explicit', - overrides: { constructors: 'off' }, - }, - ], - '@typescript-eslint/member-ordering': [ - 'error', - { - default: [ - 'public-static-field', - 'protected-static-field', - 'private-static-field', - 'public-instance-field', - 'protected-instance-field', - 'private-instance-field', - 'public-static-method', - 'protected-static-method', - 'private-static-method', - 'public-constructor', - 'public-instance-method', - 'protected-constructor', - 'protected-instance-method', - 'private-constructor', - 'private-instance-method', - ], - }, - ], - '@typescript-eslint/no-shadow': 'error', - - // permanently disable due to using other rules which do the same thing - camelcase: 'off', // redundant with @typescript-eslint/naming-convention - - // permanently disable due to improper TS handling or unnecessary for TS - // (and not covered by plugin:@typescript-eslint/eslint-recommended) - 'no-empty-function': 'off', - 'no-shadow': 'off', - 'no-unused-vars': 'off', - 'react/jsx-filename-extension': 'off', - }, - }, - { - // Test overrides - files: [...configHelpers.testFiles], - rules: { - 'no-console': 'off', - 'react/jsx-no-bind': 'off', - }, - }, - { - files: 'src/**/*.deprecated.test.{ts,tsx}', - rules: { - 'deprecation/deprecation': 'off', // the purpose of these tests - }, - }, - { - // Example overrides - files: '**/*.{Example,stories}.tsx', - rules: { - 'no-alert': 'off', - 'no-console': 'off', - }, - }, - { - files: '**/*.stories.tsx', - rules: { - // allow arrow functions in stories for now (may want to change this later since using - // constantly-mutating functions can be an anti-pattern which we may not want to demonstrate - // in our converged components docs; it happened to be allowed starting out because .stories - // files were being linted as tests) - 'react/jsx-no-bind': 'off', - }, - }, - { - // Docs overrides (excluding examples) - files: [...configHelpers.docsFiles], - rules: { - 'import/no-webpack-loader-syntax': 'off', // this is ok in docs - }, - }, - { - files: [...configHelpers.configFiles], - rules: { - 'no-console': 'off', - }, - }, - { - files: [...configHelpers.devDependenciesFiles], - rules: { - // TODO: https://github.com/microsoft/fluentui/issues/21999 - 'import/no-extraneous-dependencies': 'off', - }, - }, -]; - -// Why use `defineProperty` for `overrides`? -// -// By default, any logic in this file will be run every time the plugin is loaded (even if this -// config is not used) due to it being included by necessity in the package index file. -// These overrides include some more complex logic which should only run when requested, since it's -// more costly and can cause build errors if run in a package it wasn't designed for. -// If ESLint supported exporting a function from a config file, that would be an easy solution. -// Since that's not supported, we work around it by defining overrides as a property with getter. -// @ts-ignore -- `overrides?` is declared in `eslint.Linter.Config` but our `config` object doesn't define it until now -Object.defineProperty(config, 'overrides', { - enumerable: true, - get: getOverrides, -}); - -module.exports = config; diff --git a/packages/eslint-plugin/src/index.js b/packages/eslint-plugin/src/index.js index 925e6aa5645f25..033c21c969dc30 100644 --- a/packages/eslint-plugin/src/index.js +++ b/packages/eslint-plugin/src/index.js @@ -3,6 +3,7 @@ module.exports = { node: require('./configs/node'), 'node--legacy': require('./configs/node-legacy'), react: require('./configs/react'), + imports: require('./configs/imports'), 'react--legacy': require('./configs/react-legacy'), 'react-northstar': require('./configs/react-northstar'), }, diff --git a/packages/eslint-plugin/src/utils/configHelpers.js b/packages/eslint-plugin/src/utils/configHelpers.js index cfba0a8ede6666..d910ad5523c500 100644 --- a/packages/eslint-plugin/src/utils/configHelpers.js +++ b/packages/eslint-plugin/src/utils/configHelpers.js @@ -69,10 +69,10 @@ module.exports = { * Returns a rule configuration for [`@typescript-eslint/naming-convention`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/naming-convention.md). * This provides the ability to override *only* the interface rule without having to repeat or * lose the rest of the (very complicated) config. - * @param {boolean} prefixWithI - Whether to prefix interfaces with I + * @param {{prefixInterface: boolean}} config - Whether to prefix interfaces with I * @returns {import("eslint").Linter.RulesRecord} */ - getNamingConventionRule: prefixWithI => ({ + getNamingConventionRule: (config = { prefixInterface: false }) => ({ '@typescript-eslint/naming-convention': [ 'error', { selector: 'function', format: ['camelCase'], leadingUnderscore: 'allow' }, @@ -98,7 +98,7 @@ module.exports = { { selector: 'interface', format: ['PascalCase'], - ...(prefixWithI ? { prefix: ['I'] } : { custom: { regex: '^I[A-Z]', match: false } }), + ...(config.prefixInterface ? { prefix: ['I'] } : { custom: { regex: '^I[A-Z]', match: false } }), }, { selector: 'default',