feat: Dynamic imports for the Icons component (#14318)

* Add aria-label and twotone

* Enhance LazyIcon

* Fix tests and solve ject warnings

* Add new line

* Revert package-lock to master

* Fix failing test

* Implement icon overrides

* Fix failing storybook

* Clean up

* Improve var name

superset-frontend/jest.config.js

@@ -21,7 +21,7 @@ module.exports = {
   moduleNameMapper: {
     '\\.(css|less)$': '<rootDir>/spec/__mocks__/styleMock.js',
     '\\.(gif|ttf|eot|png)$': '<rootDir>/spec/__mocks__/fileMock.js',
-    '\\.svg$': '<rootDir>/spec/__mocks__/svgrMock.js',
+    '\\.svg$': '<rootDir>/spec/__mocks__/svgrMock.tsx',
     '^src/(.*)$': '<rootDir>/src/$1',
     '^spec/(.*)$': '<rootDir>/spec/$1',
   },

superset-frontend/spec/__mocks__/svgrMock.js → superset-frontend/spec/__mocks__/svgrMock.tsx

@@ -16,5 +16,14 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export default 'SvgrURL';
-export const ReactComponent = 'svg';
+
+import React, { SVGProps } from 'react';
+
+const SvgrMock = React.forwardRef<SVGSVGElement, SVGProps<SVGSVGElement>>(
+  (props, ref) => <svg ref={ref} {...props} />,
+);
+
+SvgrMock.displayName = 'SvgrMock';
+
+export const ReactComponent = SvgrMock;
+export default SvgrMock;

superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx

@@ -31,6 +31,7 @@ import {
 import { sliceId } from 'spec/fixtures/mockChartQueries';
 import { dashboardFilters } from 'spec/fixtures/mockDashboardFilters';
 import { dashboardWithFilter } from 'spec/fixtures/mockDashboardLayout';
+import Icons from 'src/components/Icons';
 import { FeatureFlag } from 'src/featureFlags';
 
 describe('FiltersBadge', () => {
@@ -129,7 +130,7 @@ describe('FiltersBadge', () => {
       ).toHaveText('1');
       // to look at the shape of the wrapper use:
       // console.log(wrapper.dive().debug())
-      expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
+      expect(wrapper.dive().find(Icons.AlertSolid)).toExist();
     });
   });
 
@@ -216,7 +217,7 @@ describe('FiltersBadge', () => {
       expect(
         wrapper.dive().find('[data-test="incompatible-filter-count"]'),
       ).toHaveText('1');
-      expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
+      expect(wrapper.dive().find(Icons.AlertSolid)).toExist();
     });
   });
 });

superset-frontend/spec/javascripts/explore/components/DatasourceControl_spec.jsx

@@ -24,7 +24,7 @@ import { Menu } from 'src/common/components';
 import DatasourceModal from 'src/datasource/DatasourceModal';
 import ChangeDatasourceModal from 'src/datasource/ChangeDatasourceModal';
 import DatasourceControl from 'src/explore/components/controls/DatasourceControl';
-import Icon from 'src/components/Icon';
+import Icons from 'src/components/Icons';
 import { Tooltip } from 'src/components/Tooltip';
 
 const defaultProps = {
@@ -119,8 +119,7 @@ describe('DatasourceControl', () => {
 
   it('should render health check message', () => {
     const wrapper = setup();
-    const alert = wrapper.find(Icon);
-    expect(alert.at(1).prop('name')).toBe('alert-solid');
+    expect(wrapper.find(Icons.AlertSolid)).toExist();
     const tooltip = wrapper.find(Tooltip).at(0);
     expect(tooltip.prop('title')).toBe(
       defaultProps.datasource.health_check_message,

superset-frontend/src/components/Alert/Alert.test.tsx

@@ -17,31 +17,37 @@
  * under the License.
  */
 import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import Alert, { AlertProps } from 'src/components/Alert';
 
 type AlertType = Pick<AlertProps, 'type'>;
 type AlertTypeValue = AlertType[keyof AlertType];
 
-test('renders with default props', () => {
+test('renders with default props', async () => {
   render(<Alert message="Message" />);
+
   expect(screen.getByRole('alert')).toHaveTextContent('Message');
-  expect(screen.queryByLabelText(`info icon`)).toBeInTheDocument();
-  expect(screen.queryByLabelText('close icon')).toBeInTheDocument();
+  expect(await screen.findByLabelText(`info icon`)).toBeInTheDocument();
+  expect(await screen.findByLabelText('close icon')).toBeInTheDocument();
 });
 
-test('renders each type', () => {
+test('renders each type', async () => {
   const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success'];
-  types.forEach(type => {
+  for (let i = 0; i < types.length; i += 1) {
+    const type = types[i];
     render(<Alert type={type} message="Message" />);
-    expect(screen.queryByLabelText(`${type} icon`)).toBeInTheDocument();
-  });
+    // eslint-disable-next-line no-await-in-loop
+    expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument();
+  }
 });
 
-test('renders without close button', () => {
+test('renders without close button', async () => {
   render(<Alert message="Message" closable={false} />);
-  expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument();
+
+  await waitFor(() => {
+    expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument();
+  });
 });
 
 test('disappear when closed', () => {
@@ -50,10 +56,12 @@ test('disappear when closed', () => {
   expect(screen.queryByRole('alert')).not.toBeInTheDocument();
 });
 
-test('renders without icon', () => {
+test('renders without icon', async () => {
   const type = 'info';
   render(<Alert type={type} message="Message" showIcon={false} />);
-  expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument();
+  await waitFor(() => {
+    expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument();
+  });
 });
 
 test('renders message', () => {

superset-frontend/src/components/Alert/index.tsx

@@ -22,7 +22,8 @@ import {
   AlertProps as AntdAlertProps,
 } from 'src/common/components';
 import { useTheme } from '@superset-ui/core';
-import Icon, { IconName } from 'src/components/Icon';
+import Icon from 'src/components/Icon';
+import Icons from 'src/components/Icons';
 
 export type AlertProps = PropsWithChildren<AntdAlertProps>;
 
@@ -40,23 +41,23 @@ export default function Alert(props: AlertProps) {
   const { alert, error, info, success } = colors;
 
   let baseColor = info;
-  let iconName: IconName = 'info-solid';
+  let AlertIcon = Icons.InfoSolid;
   if (type === 'error') {
     baseColor = error;
-    iconName = 'error-solid';
+    AlertIcon = Icons.ErrorSolid;
   } else if (type === 'warning') {
     baseColor = alert;
-    iconName = 'alert-solid';
+    AlertIcon = Icons.AlertSolid;
   } else if (type === 'success') {
     baseColor = success;
-    iconName = 'circle-check-solid';
+    AlertIcon = Icons.CircleCheckSolid;
   }
 
   return (
     <AntdAlert
       role="alert"
       showIcon={showIcon}
-      icon={<Icon name={iconName} aria-label={`${type} icon`} />}
+      icon={<AlertIcon aria-label={`${type} icon`} />}
       closeText={closable && <Icon name="x-small" aria-label="close icon" />}
       css={{
         padding: '6px 10px',

superset-frontend/src/components/Icons/AntdEnhanced.tsx

@@ -19,12 +19,14 @@
 
 import React from 'react';
 import * as AntdIcons from '@ant-design/icons/lib/icons';
-import Icon from './Icon';
+import { StyledIcon } from './Icon';
 import IconType from './IconType';
 
 const AntdEnhancedIcons = Object.keys(AntdIcons)
   .map(k => ({
-    [k]: (props: IconType) => <Icon component={AntdIcons[k]} {...props} />,
+    [k]: (props: IconType) => (
+      <StyledIcon component={AntdIcons[k]} {...props} />
+    ),
   }))
   .reduce((l, r) => ({ ...l, ...r }));
 

superset-frontend/src/components/Icons/Icon.tsx

@@ -17,34 +17,54 @@
  * under the License.
  */
 
-import React from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 import AntdIcon from '@ant-design/icons';
 import { styled } from '@superset-ui/core';
-import { CustomIconComponentProps } from '@ant-design/icons/lib/components/Icon';
+import { ReactComponent as TransparentIcon } from 'images/icons/transparent.svg';
 import IconType from './IconType';
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
-const EnhancedIcon = ({ iconColor, iconSize, ...rest }: IconType) => (
+const AntdIconComponent = ({ iconColor, iconSize, ...rest }: IconType) => (
   <AntdIcon viewBox={rest.viewBox || '0 0 24 24'} {...rest} />
 );
 
-const Icon = styled(EnhancedIcon)<IconType>`
+export const StyledIcon = styled(AntdIconComponent)<IconType>`
   ${({ iconColor }) => iconColor && `color: ${iconColor};`};
   font-size: ${({ iconSize, theme }) =>
-    iconSize ? `${theme.typography.sizes[iconSize]}px` : '24px'};
+    iconSize
+      ? `${theme.typography.sizes[iconSize] || theme.typography.sizes.m}px`
+      : '24px'};
 `;
 
-export const renderIcon = (
-  SVGComponent:
-    | React.ComponentClass<
-        CustomIconComponentProps | React.SVGProps<SVGSVGElement>,
-        any
-      >
-    | React.FunctionComponent<
-        CustomIconComponentProps | React.SVGProps<SVGSVGElement>
-      >
-    | undefined,
-  props: IconType,
-) => <Icon component={SVGComponent} {...props} />;
+interface IconProps extends IconType {
+  fileName: string;
+}
+
+export const Icon = (props: IconProps) => {
+  const { fileName, ...iconProps } = props;
+  const [, setLoaded] = useState(false);
+  const ImportedSVG = useRef<React.FC<React.SVGProps<SVGSVGElement>>>();
+  const name = fileName.replace('_', '-');
+
+  useEffect(() => {
+    async function importIcon(): Promise<void> {
+      ImportedSVG.current = (
+        await import(
+          `!!@svgr/webpack?-svgo,+titleProp,+ref!images/icons/${fileName}.svg`
+        )
+      ).default;
+      setLoaded(true);
+    }
+    importIcon();
+  }, [fileName, ImportedSVG]);
+
+  return (
+    <StyledIcon
+      component={ImportedSVG.current || TransparentIcon}
+      aria-label={name}
+      {...iconProps}
+    />
+  );
+};
 
 export default Icon;

superset-frontend/src/components/Icons/IconType.ts

@@ -22,6 +22,7 @@ import AntdIcon from '@ant-design/icons';
 type AntdIconType = typeof AntdIcon.defaultProps;
 type IconType = AntdIconType & {
   iconColor?: string;
+  twoToneColor?: string;
   iconSize?: 's' | 'm' | 'l' | 'xl' | 'xxl';
 };
 

superset-frontend/src/components/Icons/Icons.stories.tsx

@@ -78,6 +78,11 @@ InteractiveIcons.argTypes = {
     defaultValue: null,
     control: { type: 'select', options: palette },
   },
+  // @TODO twoToneColor is being ignored
+  twoToneColor: {
+    defaultValue: null,
+    control: { type: 'select', options: palette },
+  },
   theme: {
     table: {
       disable: true,