-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(eslint-plugin): make import/no-extraneous-dependencies work for react config #21924
Changes from all commits
6141a2d
fa7c232
c5f186f
14de40d
c955bff
84ecd83
bc4d1d3
e29b1b1
6ca077f
a42389f
7708a55
ec845e9
7c6f659
2a48550
02c338a
c6c42d9
305186e
61aa885
74d2ef8
0bcc3bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,12 +7,12 @@ | |
"scripts": { | ||
"clean": "just-scripts clean", | ||
"generate:site": "just-scripts generate:site", | ||
"lint": "eslint --ext .js,.ts --cache ." | ||
"lint": "eslint --ext .js,.ts --cache .", | ||
"type-check": "tsc -p ." | ||
}, | ||
"license": "MIT", | ||
"devDependencies": { | ||
"@fluentui/eslint-plugin": "*", | ||
"@fluentui/scripts": "^1.0.0", | ||
"@microsoft/eslint-plugin-sdl": "0.1.9" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. applied single version policy for devDeps |
||
"@fluentui/scripts": "^1.0.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needed to remove project config fallback to root this simplifies linting rules and makes our setup unified ( all packages have tsconfig and type-checking on CI ) |
||
"extends": "../../scripts/tsconfig.json", | ||
"compilerOptions": { | ||
"noEmit": true, | ||
"allowJs": true | ||
}, | ||
"include": ["pr-deploy-site.js", "just.config.ts"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "none", | ||
"comment": "chore(cra-template): remove import/resolve from lint config", | ||
"packageName": "@fluentui/cra-template", | ||
"email": "[email protected]", | ||
"dependentChangeType": "none" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "patch", | ||
"comment": "fix(eslint-plugin): make import/no-extraneous-dependencies work for react config", | ||
"packageName": "@fluentui/eslint-plugin", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "add missing dependencies", | ||
"packageName": "@fluentui/react-accordion", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "add missing dependencies", | ||
"packageName": "@fluentui/react-avatar", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "none", | ||
"comment": "chore(react-components): fix lint issues exposed by import/no-extrenaus-dependency rule", | ||
"packageName": "@fluentui/react-components", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "add react-conformance missing dependency", | ||
"packageName": "@fluentui/react-conformance-griffel", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "fix missing dependencies and lint warnings", | ||
"packageName": "@fluentui/react-tabs", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,6 +93,7 @@ | |
"@griffel/webpack-loader": "2.0.0", | ||
"@linaria/babel-preset": "3.0.0-beta.14", | ||
"@microsoft/api-extractor": "7.18.1", | ||
"@microsoft/eslint-plugin-sdl": "0.1.9", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. single version policy |
||
"@nrwl/cli": "13.8.1", | ||
"@nrwl/devkit": "13.8.1", | ||
"@nrwl/jest": "13.8.1", | ||
|
@@ -179,10 +180,10 @@ | |
"eslint": "7.25.0", | ||
"eslint-config-airbnb": "18.2.1", | ||
"eslint-config-prettier": "8.3.0", | ||
"eslint-import-resolver-typescript": "2.4.0", | ||
"eslint-import-resolver-typescript": "2.5.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bumped for bug fixes |
||
"eslint-plugin-deprecation": "1.2.1", | ||
"eslint-plugin-es": "4.1.0", | ||
"eslint-plugin-import": "2.22.1", | ||
"eslint-plugin-import": "2.25.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bumped to make properly resolve path aliases |
||
"eslint-plugin-jest": "23.20.0", | ||
"eslint-plugin-jsdoc": "^36.0.7", | ||
"eslint-plugin-jsx-a11y": "6.4.1", | ||
|
@@ -416,6 +417,14 @@ | |
"ci-info", | ||
"node-fetch" | ||
] | ||
}, | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. version group needed, because we cannot specify |
||
"packages": [ | ||
"@fluentui/react-conformance-griffel" | ||
], | ||
"dependencies": [ | ||
"@fluentui/react-conformance" | ||
] | ||
} | ||
] | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"extends": "../../scripts/tsconfig.json", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needed to remove project config fallback to root this simplifies linting rules and makes our setup unified ( all packages have tsconfig and type-checking on CI ) |
||
"compilerOptions": { | ||
"noEmit": true | ||
}, | ||
"include": ["scripts"], | ||
"exclude": ["node_modules", "template"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,36 @@ | ||
// @ts-check | ||
const path = require('path'); | ||
const configHelpers = require('../utils/configHelpers'); | ||
|
||
const gitRoot = configHelpers.findGitRoot(); | ||
|
||
/** @type {import("eslint").Linter.Config} */ | ||
const config = { | ||
root: true, | ||
extends: [ | ||
// Provides both rules and some parser options and other settings | ||
'airbnb', | ||
// 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', | ||
'import', | ||
'jest', | ||
'jsdoc', | ||
'jsx-a11y', | ||
'react', | ||
'react-hooks', | ||
], | ||
settings: { | ||
// Some config suggestions copied from https://github.com/alexgorbatchev/eslint-import-resolver-typescript#configuration | ||
'import/parsers': { | ||
'@typescript-eslint/parser': ['.ts', '.tsx'], | ||
}, | ||
'import/resolver': { | ||
// @see https://github.com/alexgorbatchev/eslint-import-resolver-typescript#configuration | ||
typescript: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this configuration was breaking |
||
// always try to resolve types under `<root>@types` directory | ||
alwaysTryTypes: true, | ||
// NOTE: For packages without a tsconfig.json, override with "project": "../../tsconfig.json" | ||
project: ['./tsconfig.json', path.join(gitRoot, 'tsconfig.json')], | ||
project: './tsconfig.json', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now every package has tsconfig - no magic resolution needs to be used |
||
}, | ||
}, | ||
jsdoc: { | ||
|
@@ -102,7 +97,6 @@ const config = { | |
'dot-notation': 'error', | ||
eqeqeq: ['error', 'always'], | ||
'guard-for-in': 'error', | ||
'import/no-extraneous-dependencies': ['error', { devDependencies: false }], | ||
'jsx-a11y/tabindex-no-positive': 'error', | ||
'no-alert': 'error', | ||
'no-bitwise': 'error', | ||
|
@@ -173,15 +167,7 @@ const config = { | |
'default-case': 'off', | ||
'func-names': 'off', | ||
'global-require': 'off', | ||
'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', | ||
|
||
'jsx-a11y/alt-text': 'off', | ||
'jsx-a11y/anchor-is-valid': 'off', | ||
'jsx-a11y/aria-activedescendant-has-tabindex': 'off', | ||
|
@@ -256,17 +242,13 @@ const config = { | |
'no-restricted-syntax': 'off', | ||
|
||
// permanently disable because we disagree with these rules | ||
'import/prefer-default-export': 'off', | ||
'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 being unnecessary or having limited benefit for TS | ||
'import/export': '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 | ||
|
@@ -277,24 +259,40 @@ const config = { | |
'react/no-unused-prop-types': 'off', | ||
'react/prefer-es6-class': 'off', | ||
|
||
'jsdoc/check-tag-names': [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no changes here - this occurs as diff because moving of lines.... |
||
'error', | ||
{ | ||
// Allow TSDoc tags @remarks and @defaultValue | ||
definedTags: ['remarks', 'defaultValue'], | ||
}, | ||
], | ||
|
||
/** | ||
* | ||
* import plugin rules | ||
* @see https://github.com/import-js/eslint-plugin-import | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. collocated all import plugins rules to one place so its easier to navigate/read |
||
*/ | ||
'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/named': 'off', | ||
'import/namespace': 'off', | ||
'import/no-named-as-default-member': 'off', | ||
|
||
'jsdoc/check-tag-names': [ | ||
'error', | ||
{ | ||
// Allow TSDoc tags @remarks and @defaultValue | ||
definedTags: ['remarks', 'defaultValue'], | ||
}, | ||
], | ||
'import/export': 'off', | ||
}, | ||
}; | ||
|
||
|
@@ -406,7 +404,8 @@ const getOverrides = () => [ | |
{ | ||
files: [...configHelpers.devDependenciesFiles], | ||
rules: { | ||
'import/no-extraneous-dependencies': ['error', { packageDir: ['.', gitRoot] }], | ||
// TODO: https://github.com/microsoft/fluentui/issues/21999 | ||
'import/no-extraneous-dependencies': 'off', | ||
}, | ||
}, | ||
]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
{ | ||
"copyTo": { | ||
"unstable": ["./src/unstable/package.json"] | ||
"unstable/package.json": "./src/unstable/package.json__tmpl__" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. leveraging new API : copy and rename to destination key There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. package.json is using |
||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added type checking