From 8e88caa0299a1502326ac7c9c2e5e7cac18f5bcb Mon Sep 17 00:00:00 2001 From: Philipp Hugenroth Date: Fri, 5 May 2023 15:37:47 +0200 Subject: [PATCH] Simplify PR after review Co-authored-by: Patrik Oldsberg Signed-off-by: Philipp Hugenroth --- .eslintrc.js | 6 -- packages/cli/config/eslint-factory.js | 10 +++ packages/theme/src/base/index.ts | 1 + packages/theme/src/unified/UnifiedTheme.tsx | 9 +-- .../src/unified/UnifiedThemeProvider.tsx | 13 +--- packages/theme/src/v4/load.ts | 74 ------------------- .../src/test-utils.tsx | 2 - 7 files changed, 14 insertions(+), 101 deletions(-) delete mode 100644 packages/theme/src/v4/load.ts diff --git a/.eslintrc.js b/.eslintrc.js index 5c1012e501..1616404a6f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -28,12 +28,6 @@ module.exports = { onNonMatchingHeader: 'replace', }, ], - 'no-restricted-imports': [ - 'error', - { - patterns: ['@mui/*/*/*'], - }, - ], 'no-restricted-syntax': [ 'error', { diff --git a/packages/cli/config/eslint-factory.js b/packages/cli/config/eslint-factory.js index dde853fdff..6888e0ff67 100644 --- a/packages/cli/config/eslint-factory.js +++ b/packages/cli/config/eslint-factory.js @@ -24,6 +24,7 @@ const { join: joinPath } = require('path'); * - `tsRules`: Additional ESLint rules to apply to TypeScript * - `testRules`: Additional ESLint rules to apply to tests * - `restrictedImports`: Additional paths to add to no-restricted-imports + * - `restrictedImportsPattern`: Additional patterns to add to no-restricted-imports * - `restrictedSrcImports`: Additional paths to add to no-restricted-imports in src files * - `restrictedTestImports`: Additional paths to add to no-restricted-imports in test files * - `restrictedSyntax`: Additional patterns to add to no-restricted-syntax @@ -44,6 +45,7 @@ function createConfig(dir, extraConfig = {}) { testRules, restrictedImports, + restrictedImportsPatterns, restrictedSrcImports, restrictedTestImports, restrictedSyntax, @@ -114,6 +116,7 @@ function createConfig(dir, extraConfig = {}) { '*.test*', '**/__testUtils__/**', '**/__mocks__/**', + ...(restrictedImportsPatterns ?? []), ], }, ], @@ -208,9 +211,16 @@ function createConfigForRole(dir, role, extraConfig = {}) { name: '@material-ui/icons/', // because this is possible too ._. message: "Please import '@material-ui/icons/' instead.", }, + { + // https://mui.com/material-ui/guides/minimizing-bundle-size/ + name: '@mui/material', + message: "Please import '@mui/material/...' instead.", + }, ...require('module').builtinModules, ...(extraConfig.restrictedImports ?? []), ], + // https://mui.com/material-ui/guides/minimizing-bundle-size/ + restrictedImportsPatterns: ['@mui/*/*/*'], tsRules: { 'react/prop-types': 0, ...extraConfig.tsRules, diff --git a/packages/theme/src/base/index.ts b/packages/theme/src/base/index.ts index 45b8326f2e..4bb80aa42b 100644 --- a/packages/theme/src/base/index.ts +++ b/packages/theme/src/base/index.ts @@ -19,6 +19,7 @@ export type { BaseThemeOptionsInput } from './createBaseThemeOptions'; export { colorVariants, genPageTheme, pageTheme, shapes } from './pageTheme'; export { palettes } from './palettes'; export type { + BackstageThemeAdditions, BackstagePaletteAdditions, PageTheme, PageThemeSelector, diff --git a/packages/theme/src/unified/UnifiedTheme.tsx b/packages/theme/src/unified/UnifiedTheme.tsx index c878cfd5e1..0c5f80357a 100644 --- a/packages/theme/src/unified/UnifiedTheme.tsx +++ b/packages/theme/src/unified/UnifiedTheme.tsx @@ -32,7 +32,6 @@ import { PageTheme } from '../base/types'; import { defaultComponentThemes } from '../v5'; import { createBaseThemeOptions } from '../base/createBaseThemeOptions'; import { UnifiedTheme } from './types'; -import { maybeLoadMui4Styles } from '../v4/load'; export class UnifiedThemeHolder implements UnifiedTheme { #themes = new Map(); @@ -97,12 +96,6 @@ export function createUnifiedThemeFromV4( options: ThemeOptionsV4, ): UnifiedTheme { const v5Theme = adaptV4Theme(options as any); - - const mui4Styles = maybeLoadMui4Styles(); - if (!mui4Styles) { - return new UnifiedThemeHolder(undefined, v5Theme); - } - - const v4Theme = mui4Styles.createTheme(options); + const v4Theme = createTheme(options); return new UnifiedThemeHolder(v4Theme, v5Theme); } diff --git a/packages/theme/src/unified/UnifiedThemeProvider.tsx b/packages/theme/src/unified/UnifiedThemeProvider.tsx index 88cc1d9457..c7054e0e55 100644 --- a/packages/theme/src/unified/UnifiedThemeProvider.tsx +++ b/packages/theme/src/unified/UnifiedThemeProvider.tsx @@ -21,9 +21,8 @@ import { StyledEngineProvider, ThemeProvider as Mui5Provider, } from '@mui/material/styles'; -import Mui5CssBaseline from '@mui/material/CssBaseline'; +import CSSBaseline from '@mui/material/CssBaseline'; import { UnifiedTheme } from './types'; -import { CssBaseline } from '@material-ui/core'; /** * Props for {@link UnifiedThemeProvider}. @@ -51,15 +50,7 @@ export function UnifiedThemeProvider( let cssBaseline: JSX.Element | undefined = undefined; if (!noCssBaseline) { - if (v5Theme) { - cssBaseline = ; - } else if (v4Theme) { - /* const CssBaseline = maybeLoadMui4CssBaseline(); - if (!CssBaseline) { - throw new Error('Failed to load MUI 4 CssBaseline component'); - } */ - cssBaseline = ; - } + cssBaseline = ; } let result = ( diff --git a/packages/theme/src/v4/load.ts b/packages/theme/src/v4/load.ts deleted file mode 100644 index 899aa8af93..0000000000 --- a/packages/theme/src/v4/load.ts +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright 2023 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Loads MUI v4 styles if it is available. - * - * This should always be used to access MUI v4 at runtime in this - * package, to ensure that it's not brought into the bundle unnecessarily. - * - * @internal - */ -export function maybeLoadMui4Styles(): - | typeof import('@material-ui/core/styles') - | undefined { - if ( - __webpack_modules__[ - require.resolveWeak('@material-ui/core/styles') as number - ] - ) { - return __webpack_modules__[ - require.resolveWeak('@material-ui/core/styles') as number - ] as typeof import('@material-ui/core/styles'); - } - if (__webpack_modules__[require.resolveWeak('@material-ui/core') as number]) { - return __webpack_modules__[ - require.resolveWeak('@material-ui/core') as number - ] as typeof import('@material-ui/core'); - } - - return undefined; -} - -/** - * Loads the MUI v4 CssBaseline component if it is available. - * - * This should always be used to access MUI v4 at runtime in this - * package, to ensure that it's not brought into the bundle unnecessarily. - * - * @internal - */ -export function maybeLoadMui4CssBaseline(): - | typeof import('@material-ui/core/CssBaseline')['default'] - | undefined { - if ( - __webpack_modules__[ - require.resolveWeak('@material-ui/core/CssBaseline') as number - ] - ) { - const m = __webpack_modules__[ - require.resolveWeak('@material-ui/core/CssBaseline') as number - ] as typeof import('@material-ui/core/CssBaseline'); - return m.default; - } - if (__webpack_modules__[require.resolveWeak('@material-ui/core') as number]) { - const m = __webpack_modules__[ - require.resolveWeak('@material-ui/core') as number - ] as typeof import('@material-ui/core'); - return m.CssBaseline; - } - return undefined; -} diff --git a/plugins/techdocs-addons-test-utils/src/test-utils.tsx b/plugins/techdocs-addons-test-utils/src/test-utils.tsx index c7a3992281..a6e83262fb 100644 --- a/plugins/techdocs-addons-test-utils/src/test-utils.tsx +++ b/plugins/techdocs-addons-test-utils/src/test-utils.tsx @@ -278,8 +278,6 @@ export class TechDocsAddonTester { render(this.build()); }); - await waitFor(() => screen.getByText('Documentation')); - const shadowHost = await screen.findByTestId('techdocs-native-shadowroot'); return {