SafeDynamicImport: Updates so that it does not act as an ErrorBoundary (#20170)

* SafeDynamicImport: Fixes so that it shows different messages depending on error

* Refactor: Fixes type error

* Refactor: Adds grafana constant to error message

* Refactor: Renames components and adds exports

* Refactor: Uses react-loader instead

* Refactor: Updates after PR comments

* Tests: Adds tests for loadComponentHandler

package.json

@@ -198,6 +198,7 @@
     "@babel/polyfill": "7.6.0",
     "@grafana/slate-react": "0.22.9-grafana",
     "@torkelo/react-select": "2.4.1",
+    "@types/react-loadable": "5.5.2",
     "angular": "1.6.9",
     "angular-bindonce": "0.3.1",
     "angular-native-dragdrop": "1.2.2",
@@ -234,6 +235,7 @@
     "react-dom": "16.8.6",
     "react-grid-layout": "0.16.6",
     "react-highlight-words": "0.11.0",
+    "react-loadable": "5.5.0",
     "react-popper": "1.3.3",
     "react-redux": "7.1.1",
     "react-sizeme": "2.5.2",

packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx

@@ -1,19 +1,18 @@
 import React, { PureComponent, ReactNode } from 'react';
 import { Alert } from '../Alert/Alert';
-import { css } from 'emotion';
+import { ErrorWithStack } from './ErrorWithStack';
-import { stylesFactory } from '../../themes';
 
-interface ErrorInfo {
+export interface ErrorInfo {
   componentStack: string;
 }
 
-interface RenderProps {
+export interface ErrorBoundaryApi {
   error: Error | null;
   errorInfo: ErrorInfo | null;
 }
 
 interface Props {
-  children: (r: RenderProps) => ReactNode;
+  children: (r: ErrorBoundaryApi) => ReactNode;
 }
 
 interface State {
@@ -45,13 +44,6 @@ export class ErrorBoundary extends PureComponent<Props, State> {
   }
 }
 
-const getStyles = stylesFactory(() => {
-  return css`
-    width: 500px;
-    margin: 64px auto;
-  `;
-});
-
 interface WithAlertBoxProps {
   title?: string;
   children: ReactNode;
@@ -84,18 +76,9 @@ export class ErrorBoundaryAlert extends PureComponent<WithAlertBoxProps> {
                 </details>
               </Alert>
             );
-          } else {
-            return (
-              <div className={getStyles()}>
-                <h2>{title}</h2>
-                <details style={{ whiteSpace: 'pre-wrap' }}>
-                  {error && error.toString()}
-                  <br />
-                  {errorInfo.componentStack}
-                </details>
-              </div>
-            );
           }
+
+          return <ErrorWithStack title={title || ''} error={error} errorInfo={errorInfo} />;
         }}
       </ErrorBoundary>
     );

packages/grafana-ui/src/components/ErrorBoundary/ErrorWithStack.tsx

@@ -0,0 +1,28 @@
+import React, { FunctionComponent } from 'react';
+import { ErrorBoundaryApi } from './ErrorBoundary';
+import { stylesFactory } from '../../themes';
+import { css } from 'emotion';
+
+const getStyles = stylesFactory(() => {
+  return css`
+    width: 500px;
+    margin: 64px auto;
+  `;
+});
+
+export interface Props extends ErrorBoundaryApi {
+  title: string;
+}
+
+export const ErrorWithStack: FunctionComponent<Props> = ({ error, errorInfo, title }) => (
+  <div className={getStyles()}>
+    <h2>{title}</h2>
+    <details style={{ whiteSpace: 'pre-wrap' }}>
+      {error && error.toString()}
+      <br />
+      {errorInfo && errorInfo.componentStack}
+    </details>
+  </div>
+);
+
+ErrorWithStack.displayName = 'ErrorWithStack';

packages/grafana-ui/src/components/index.ts

@@ -87,6 +87,7 @@ export { TransformationsEditor } from './TransformersUI/TransformationsEditor';
 export { JSONFormatter } from './JSONFormatter/JSONFormatter';
 export { JsonExplorer } from './JSONFormatter/json_explorer/json_explorer';
 export { ErrorBoundary, ErrorBoundaryAlert } from './ErrorBoundary/ErrorBoundary';
+export { ErrorWithStack } from './ErrorBoundary/ErrorWithStack';
 export { AlphaNotice } from './AlphaNotice/AlphaNotice';
 export { DataSourceHttpSettings } from './DataSourceSettings/DataSourceHttpSettings';
 export { Spinner } from './Spinner/Spinner';

public/app/core/components/DynamicImports/ErrorLoadingChunk.tsx

@@ -0,0 +1,35 @@
+import React, { FunctionComponent } from 'react';
+import { Button, stylesFactory } from '@grafana/ui';
+import { css } from 'emotion';
+
+const getStyles = stylesFactory(() => {
+  return css`
+    width: 508px;
+    margin: 128px auto;
+  `;
+});
+
+interface Props {
+  error: Error | null;
+}
+
+export const ErrorLoadingChunk: FunctionComponent<Props> = ({ error }) => (
+  <div className={getStyles()}>
+    <h2>Unable to find application file</h2>
+    <br />
+    <h2 className="page-heading">Grafana has likely been updated. Please try reloading the page.</h2>
+    <br />
+    <div className="gf-form-group">
+      <Button size="md" variant="secondary" icon="fa fa-repeat" onClick={() => window.location.reload()}>
+        Reload
+      </Button>
+    </div>
+    <details style={{ whiteSpace: 'pre-wrap' }}>
+      {error && error.message ? error.message : 'Unexpected error occurred'}
+      <br />
+      {error && error.stack ? error.stack : null}
+    </details>
+  </div>
+);
+
+ErrorLoadingChunk.displayName = 'ErrorLoadingChunk';

public/app/core/components/DynamicImports/LoadingChunkPlaceHolder.tsx

@@ -0,0 +1,10 @@
+import React, { FunctionComponent } from 'react';
+import { LoadingPlaceholder } from '@grafana/ui';
+
+export const LoadingChunkPlaceHolder: FunctionComponent = React.memo(() => (
+  <div className="preloader">
+    <LoadingPlaceholder text={'Loading...'} />
+  </div>
+));
+
+LoadingChunkPlaceHolder.displayName = 'LoadingChunkPlaceHolder';

public/app/core/components/DynamicImports/SafeDynamicImport.test.tsx

@@ -0,0 +1,36 @@
+import React from 'react';
+import { loadComponentHandler } from './SafeDynamicImport';
+import { ErrorLoadingChunk } from './ErrorLoadingChunk';
+import { LoadingChunkPlaceHolder } from './LoadingChunkPlaceHolder';
+
+describe('loadComponentHandler', () => {
+  describe('when there is no error and pastDelay is false', () => {
+    it('then it should return null', () => {
+      const error: Error = null;
+      const pastDelay = false;
+      const element = loadComponentHandler({ error, pastDelay });
+
+      expect(element).toBe(null);
+    });
+  });
+
+  describe('when there is an error', () => {
+    it('then it should return ErrorLoadingChunk', () => {
+      const error: Error = new Error('Some chunk failed to load');
+      const pastDelay = false;
+      const element = loadComponentHandler({ error, pastDelay });
+
+      expect(element).toEqual(<ErrorLoadingChunk error={error} />);
+    });
+  });
+
+  describe('when loading is taking more then default delay of 200ms', () => {
+    it('then it should return LoadingChunkPlaceHolder', () => {
+      const error: Error = null;
+      const pastDelay = true;
+      const element = loadComponentHandler({ error, pastDelay });
+
+      expect(element).toEqual(<LoadingChunkPlaceHolder />);
+    });
+  });
+});

public/app/core/components/DynamicImports/SafeDynamicImport.tsx

@@ -0,0 +1,27 @@
+import React from 'react';
+import Loadable from 'react-loadable';
+import { LoadingChunkPlaceHolder } from './LoadingChunkPlaceHolder';
+import { ErrorLoadingChunk } from './ErrorLoadingChunk';
+
+export const loadComponentHandler = (props: { error: Error; pastDelay: boolean }) => {
+  const { error, pastDelay } = props;
+
+  if (error) {
+    return <ErrorLoadingChunk error={error} />;
+  }
+
+  if (pastDelay) {
+    return <LoadingChunkPlaceHolder />;
+  }
+
+  return null;
+};
+
+export const SafeDynamicImport = (importStatement: Promise<any>) => ({ ...props }) => {
+  const LoadableComponent = Loadable({
+    loader: () => importStatement,
+    loading: loadComponentHandler,
+  });
+
+  return <LoadableComponent {...props} />;
+};

public/app/core/components/SafeDynamicImport.tsx

@@ -1,52 +0,0 @@
-import React, { lazy, Suspense, FunctionComponent } from 'react';
-import { cx, css } from 'emotion';
-import { LoadingPlaceholder, ErrorBoundary, Button } from '@grafana/ui';
-
-export const LoadingChunkPlaceHolder: FunctionComponent = () => (
-  <div className={cx('preloader')}>
-    <LoadingPlaceholder text={'Loading...'} />
-  </div>
-);
-
-function getAlertPageStyle() {
-  return css`
-    width: 508px;
-    margin: 128px auto;
-  `;
-}
-
-export const SafeDynamicImport = (importStatement: Promise<any>) => ({ ...props }) => {
-  const LazyComponent = lazy(() => importStatement);
-  return (
-    <ErrorBoundary>
-      {({ error, errorInfo }) => {
-        if (!errorInfo) {
-          return (
-            <Suspense fallback={<LoadingChunkPlaceHolder />}>
-              <LazyComponent {...props} />
-            </Suspense>
-          );
-        }
-
-        return (
-          <div className={getAlertPageStyle()}>
-            <h2>Unable to find application file</h2>
-            <br />
-            <h2 className="page-heading">Grafana has likely been updated. Please try reloading the page.</h2>
-            <br />
-            <div className="gf-form-group">
-              <Button size={'md'} variant={'secondary'} icon="fa fa-repeat" onClick={() => window.location.reload()}>
-                Reload
-              </Button>
-            </div>
-            <details style={{ whiteSpace: 'pre-wrap' }}>
-              {error && error.toString()}
-              <br />
-              {errorInfo.componentStack}
-            </details>
-          </div>
-        );
-      }}
-    </ErrorBoundary>
-  );
-};

public/app/routes/routes.ts

@@ -7,11 +7,11 @@ import FolderDashboardsCtrl from 'app/features/folders/FolderDashboardsCtrl';
 import DashboardImportCtrl from 'app/features/manage-dashboards/DashboardImportCtrl';
 import LdapPage from 'app/features/admin/ldap/LdapPage';
 import config from 'app/core/config';
-import { route, ILocationProvider } from 'angular';
+import { ILocationProvider, route } from 'angular';
 // Types
 import { DashboardRouteInfo } from 'app/types';
 import { LoginPage } from 'app/core/components/Login/LoginPage';
-import { SafeDynamicImport } from '../core/components/SafeDynamicImport';
+import { SafeDynamicImport } from '../core/components/DynamicImports/SafeDynamicImport';
 
 /** @ngInject */
 export function setupAngularRoutes($routeProvider: route.IRouteProvider, $locationProvider: ILocationProvider) {

yarn.lock

@@ -3587,6 +3587,14 @@
   dependencies:
     "@types/react" "*"
 
+"@types/react-loadable@5.5.2":
+  version "5.5.2"
+  resolved "https://registry.yarnpkg.com/@types/react-loadable/-/react-loadable-5.5.2.tgz#ea7c3bf3a137d6349b766e732842d0cdf0bc3dc2"
+  integrity sha512-aTgaRAgUE/mjjozu0EAv7RolGvd4rqgP8janJbxPtQow5m1O2XaaxSct8foUpZCbwRkwJ+ysPJti2F4krdg9PQ==
+  dependencies:
+    "@types/react" "*"
+    "@types/webpack" "*"
+
 "@types/react-redux@7.1.2":
   version "7.1.2"
   resolved "https://registry.yarnpkg.com/@types/react-redux/-/react-redux-7.1.2.tgz#02303b77d87e54f327c09507cf80ee3ca3063898"
@@ -16506,7 +16514,7 @@ prop-types-exact@^1.2.0:
     object.assign "^4.1.0"
     reflect.ownkeys "^0.2.0"
 
-prop-types@15.7.2, prop-types@15.x, prop-types@^15.5.10, prop-types@^15.5.8, prop-types@^15.6.0, prop-types@^15.6.1, prop-types@^15.6.2, prop-types@^15.7.2:
+prop-types@15.7.2, prop-types@15.x, prop-types@^15.5.0, prop-types@^15.5.10, prop-types@^15.5.8, prop-types@^15.6.0, prop-types@^15.6.1, prop-types@^15.6.2, prop-types@^15.7.2:
   version "15.7.2"
   resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5"
   integrity sha512-8QQikdH7//R2vurIJSutZ1smHYTcLpRWEOlHnzcWHmBYrOGUysKwSsrC89BCiFj3CbrfJ/nXFdJepOVrY1GCHQ==
@@ -17168,6 +17176,13 @@ react-lifecycles-compat@^3.0.4:
   resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"
   integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==
 
+react-loadable@5.5.0:
+  version "5.5.0"
+  resolved "https://registry.yarnpkg.com/react-loadable/-/react-loadable-5.5.0.tgz#582251679d3da86c32aae2c8e689c59f1196d8c4"
+  integrity sha512-C8Aui0ZpMd4KokxRdVAm2bQtI03k2RMRNzOB+IipV3yxFTSVICv7WoUr5L9ALB5BmKO1iHgZtWM8EvYG83otdg==
+  dependencies:
+    prop-types "^15.5.0"
+
 react-popper-tooltip@^2.8.3:
   version "2.9.1"
   resolved "https://registry.yarnpkg.com/react-popper-tooltip/-/react-popper-tooltip-2.9.1.tgz#cc602c89a937aea378d9e2675b1ce62805beb4f6"