From 0a00032e3f4ca845ed12df0ae19bacbf79dab3fd Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 14 May 2020 19:38:22 +0200 Subject: [PATCH 1/3] [eslint-plugin-material-ui] Remove restricted-path-imports --- .eslintrc.js | 7 +++- .../typography/ResponsiveFontSizesChart.js | 2 +- .../minimizing-bundle-size.md | 15 ++++++++ packages/eslint-plugin-material-ui/README.md | 17 ++++++++- .../eslint-plugin-material-ui/src/index.js | 1 - .../src/rules/restricted-path-imports.js | 28 -------------- .../src/rules/restricted-path-imports.test.js | 38 ------------------- 7 files changed, 37 insertions(+), 71 deletions(-) delete mode 100644 packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js delete mode 100644 packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.test.js diff --git a/.eslintrc.js b/.eslintrc.js index 11fa2a90ad2fe2..5c2000877e639e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -41,6 +41,12 @@ module.exports = { // Airbnb use error 'no-param-reassign': 'off', 'no-prototype-builtins': 'off', + 'no-restricted-imports': [ + 'error', + { + patterns: ['@material-ui/*/*/*', '!@material-ui/core/test-utils/*'], + }, + ], 'nonblock-statement-body-position': 'error', // Airbnb restricts isNaN and isFinite which are necessary for IE 11 // we have to be disciplined about the usage and ensure the Number type for its @@ -55,7 +61,6 @@ module.exports = { 'jsx-a11y/no-autofocus': 'off', // We are a library, people do what they want. 'material-ui/docgen-ignore-before-comment': 'error', - 'material-ui/restricted-path-imports': 'error', // This rule is great for raising people awareness of what a key is and how it works. 'react/no-array-index-key': 'off', diff --git a/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js b/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js index 0b6df3db86f217..8672162ca945c2 100644 --- a/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js +++ b/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js @@ -1,4 +1,4 @@ -/* eslint-disable material-ui/restricted-path-imports */ +/* eslint-disable no-restricted-imports */ import React from 'react'; import { convertLength } from '@material-ui/core/styles/cssUtils'; import { makeStyles, createMuiTheme, responsiveFontSizes } from '@material-ui/core/styles'; diff --git a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md index 34a8a730227940..31e05306fc51f5 100644 --- a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md +++ b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md @@ -71,6 +71,21 @@ import TabIndicator from '@material-ui/core/Tabs/TabIndicator'; // ^^^^^^^^^^^^ 3rd level ``` +If you're using `eslint` you can catch problematic imports with the [`no-restricted-imports` rule](https://eslint.org/docs/rules/no-restricted-imports). The following `.eslintrc` configuration will restricted problematics imports from `@material-ui` packages: + +```json +{ + "rules": { + "no-restricted-imports": [ + "error", + { + "patterns": ["@material-ui/*/*/*", "!@material-ui/core/test-utils/*"] + } + ] + } +} +``` + ### Option 2 This option provides the best User Experience and Developer Experience: diff --git a/packages/eslint-plugin-material-ui/README.md b/packages/eslint-plugin-material-ui/README.md index 301997237f9254..2b503947e0710b 100644 --- a/packages/eslint-plugin-material-ui/README.md +++ b/packages/eslint-plugin-material-ui/README.md @@ -7,7 +7,7 @@ Custom eslint rules for Material-UI. - `disallow-active-element-as-key-event-target` - `docgen-ignore-before-comment` - `no-hardcoded-labels` -- `restricted-path-imports` +- ~~`restricted-path-imports`~~ ### disallow-active-element-as-key-event-target @@ -26,4 +26,17 @@ The docs are translated via crowdin, we prefer to use `t` from the redux store. ### restricted-path-imports -Prevent the import of modules at a level depth strictly over 1. +Removed in favor of [`no-restricted-imports`](https://eslint.org/docs/rules/no-restricted-imports) using the following configuration: + +```json +{ + "rules": { + "no-restricted-imports": [ + "error", + { + "patterns": ["@material-ui/*/*/*", "!@material-ui/core/test-utils/*"] + } + ] + } +} +``` diff --git a/packages/eslint-plugin-material-ui/src/index.js b/packages/eslint-plugin-material-ui/src/index.js index 5cbc9ef957ef12..8bfc6c79735012 100644 --- a/packages/eslint-plugin-material-ui/src/index.js +++ b/packages/eslint-plugin-material-ui/src/index.js @@ -3,5 +3,4 @@ module.exports.rules = { 'disallow-active-element-as-key-event-target': require('./rules/disallow-active-element-as-key-event-target'), 'docgen-ignore-before-comment': require('./rules/docgen-ignore-before-comment'), 'no-hardcoded-labels': require('./rules/no-hardcoded-labels'), - 'restricted-path-imports': require('./rules/restricted-path-imports'), }; diff --git a/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js b/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js deleted file mode 100644 index 158123f02afe46..00000000000000 --- a/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js +++ /dev/null @@ -1,28 +0,0 @@ -// See tests for valid examples. -// Rationale: Importing anything deeper will use the commonJS build -module.exports = (context) => { - return { - ImportDeclaration(node) { - const { source } = node; - const folders = source.value.split('/'); - const shouldRestrictImportPath = - folders[0] === '@material-ui' && - // test-utils are fine. it doesn't matter whether we use cjs or esm since they're not deployed to the web - !source.value.startsWith('@material-ui/core/test-utils'); - if (!shouldRestrictImportPath) { - return; - } - - // @namespace/packageName is first level - const level = source.value.split('/').length - 1; - - if (level > 2) { - const preferredImport = folders.slice(0, 3).join('/'); - context.report({ - node, - message: `Only second level path imports are allowed. Prefer to import from '${preferredImport}'.`, - }); - } - }, - }; -}; diff --git a/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.test.js b/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.test.js deleted file mode 100644 index 84920185c61bb9..00000000000000 --- a/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.test.js +++ /dev/null @@ -1,38 +0,0 @@ -const eslint = require('eslint'); -const rule = require('./restricted-path-imports'); - -const ruleTester = new eslint.RuleTester({ - parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, -}); -ruleTester.run('restricted-path-imports', rule, { - valid: [ - "import { Button } from '@material-ui/core'", - "import Button from '@material-ui/core/Button'", - "import { withStyles } from '@material-ui/core/styles'", - "import { blue } from '@material-ui/core/colors'", - "import * as colors from '@material-ui/core/colors'", - "import * as colors from '@another/core/styles/withStyles'", - "import describeConformance from '@material-ui/core/test-utils/describeConformance'", - "import describeConformance from '@another/core/test-utils/describeConformance'", - ], - invalid: [ - { - code: "import withStyles from '@material-ui/core/styles/withStyles'", - errors: [ - { - message: - "Only second level path imports are allowed. Prefer to import from '@material-ui/core/styles'.", - }, - ], - }, - { - code: "import { capitalize } from '@material-ui/core/utils/helpers'", - errors: [ - { - message: - "Only second level path imports are allowed. Prefer to import from '@material-ui/core/utils'.", - }, - ], - }, - ], -}); From 66017aa623eceb406fb88956596420e49abf9669 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Fri, 15 May 2020 10:52:44 +0200 Subject: [PATCH 2/3] Update docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md Co-authored-by: Josh Wooding <12938082+joshwooding@users.noreply.github.com> --- .../guides/minimizing-bundle-size/minimizing-bundle-size.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md index 31e05306fc51f5..23fc95498909d2 100644 --- a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md +++ b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md @@ -71,7 +71,7 @@ import TabIndicator from '@material-ui/core/Tabs/TabIndicator'; // ^^^^^^^^^^^^ 3rd level ``` -If you're using `eslint` you can catch problematic imports with the [`no-restricted-imports` rule](https://eslint.org/docs/rules/no-restricted-imports). The following `.eslintrc` configuration will restricted problematics imports from `@material-ui` packages: +If you're using `eslint` you can catch problematic imports with the [`no-restricted-imports` rule](https://eslint.org/docs/rules/no-restricted-imports). The following `.eslintrc` configuration will highlight problematic imports from `@material-ui` packages: ```json { From 5cb726377054ef9a5dcff0c769aac619326d6a3e Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Fri, 15 May 2020 11:06:59 +0200 Subject: [PATCH 3/3] Add explainer to disable directive --- .../customization/typography/ResponsiveFontSizesChart.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js b/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js index 8672162ca945c2..7092b6b3ac81ac 100644 --- a/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js +++ b/docs/src/pages/customization/typography/ResponsiveFontSizesChart.js @@ -1,5 +1,7 @@ -/* eslint-disable no-restricted-imports */ import React from 'react'; +// import of a small, pure module in a private demo +// bundle size and module duplication is negligible +/* eslint-disable-next-line no-restricted-imports */ import { convertLength } from '@material-ui/core/styles/cssUtils'; import { makeStyles, createMuiTheme, responsiveFontSizes } from '@material-ui/core/styles'; import {