From 54602f16a8b0feb35c873aa048aeda54d0bd927f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Wed, 6 Nov 2019 11:04:27 +0100 Subject: [PATCH] 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 | 2 + .../ErrorBoundary/ErrorBoundary.tsx | 29 +++-------- .../ErrorBoundary/ErrorWithStack.tsx | 28 ++++++++++ packages/grafana-ui/src/components/index.ts | 1 + .../DynamicImports/ErrorLoadingChunk.tsx | 35 +++++++++++++ .../LoadingChunkPlaceHolder.tsx | 10 ++++ .../DynamicImports/SafeDynamicImport.test.tsx | 36 +++++++++++++ .../DynamicImports/SafeDynamicImport.tsx | 27 ++++++++++ .../app/core/components/SafeDynamicImport.tsx | 52 ------------------- public/app/routes/routes.ts | 4 +- yarn.lock | 17 +++++- 11 files changed, 163 insertions(+), 78 deletions(-) create mode 100644 packages/grafana-ui/src/components/ErrorBoundary/ErrorWithStack.tsx create mode 100644 public/app/core/components/DynamicImports/ErrorLoadingChunk.tsx create mode 100644 public/app/core/components/DynamicImports/LoadingChunkPlaceHolder.tsx create mode 100644 public/app/core/components/DynamicImports/SafeDynamicImport.test.tsx create mode 100644 public/app/core/components/DynamicImports/SafeDynamicImport.tsx delete mode 100644 public/app/core/components/SafeDynamicImport.tsx diff --git a/package.json b/package.json index e66b8d23233..b28b6564986 100644 --- a/package.json +++ b/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", diff --git a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx b/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx index ba0c935767c..4ed74da0c41 100644 --- a/packages/grafana-ui/src/components/ErrorBoundary/ErrorBoundary.tsx +++ b/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 { stylesFactory } from '../../themes'; +import { ErrorWithStack } from './ErrorWithStack'; -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 { } } -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 { ); - } else { - return ( -
-

{title}

-
- {error && error.toString()} -
- {errorInfo.componentStack} -
-
- ); } + + return ; }} ); diff --git a/packages/grafana-ui/src/components/ErrorBoundary/ErrorWithStack.tsx b/packages/grafana-ui/src/components/ErrorBoundary/ErrorWithStack.tsx new file mode 100644 index 00000000000..e70246b1217 --- /dev/null +++ b/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 = ({ error, errorInfo, title }) => ( +
+

{title}

+
+ {error && error.toString()} +
+ {errorInfo && errorInfo.componentStack} +
+
+); + +ErrorWithStack.displayName = 'ErrorWithStack'; diff --git a/packages/grafana-ui/src/components/index.ts b/packages/grafana-ui/src/components/index.ts index 6814b4fc4a9..3cbd36056ef 100644 --- a/packages/grafana-ui/src/components/index.ts +++ b/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'; diff --git a/public/app/core/components/DynamicImports/ErrorLoadingChunk.tsx b/public/app/core/components/DynamicImports/ErrorLoadingChunk.tsx new file mode 100644 index 00000000000..73bdf1bb0c1 --- /dev/null +++ b/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 = ({ error }) => ( +
+

Unable to find application file

+
+

Grafana has likely been updated. Please try reloading the page.

+
+
+ +
+
+ {error && error.message ? error.message : 'Unexpected error occurred'} +
+ {error && error.stack ? error.stack : null} +
+
+); + +ErrorLoadingChunk.displayName = 'ErrorLoadingChunk'; diff --git a/public/app/core/components/DynamicImports/LoadingChunkPlaceHolder.tsx b/public/app/core/components/DynamicImports/LoadingChunkPlaceHolder.tsx new file mode 100644 index 00000000000..a5813a03252 --- /dev/null +++ b/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(() => ( +
+ +
+)); + +LoadingChunkPlaceHolder.displayName = 'LoadingChunkPlaceHolder'; diff --git a/public/app/core/components/DynamicImports/SafeDynamicImport.test.tsx b/public/app/core/components/DynamicImports/SafeDynamicImport.test.tsx new file mode 100644 index 00000000000..2399bfff21b --- /dev/null +++ b/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(); + }); + }); + + 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(); + }); + }); +}); diff --git a/public/app/core/components/DynamicImports/SafeDynamicImport.tsx b/public/app/core/components/DynamicImports/SafeDynamicImport.tsx new file mode 100644 index 00000000000..81a15a67ffa --- /dev/null +++ b/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 ; + } + + if (pastDelay) { + return ; + } + + return null; +}; + +export const SafeDynamicImport = (importStatement: Promise) => ({ ...props }) => { + const LoadableComponent = Loadable({ + loader: () => importStatement, + loading: loadComponentHandler, + }); + + return ; +}; diff --git a/public/app/core/components/SafeDynamicImport.tsx b/public/app/core/components/SafeDynamicImport.tsx deleted file mode 100644 index 11eba3b7ef7..00000000000 --- a/public/app/core/components/SafeDynamicImport.tsx +++ /dev/null @@ -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 = () => ( -
- -
-); - -function getAlertPageStyle() { - return css` - width: 508px; - margin: 128px auto; - `; -} - -export const SafeDynamicImport = (importStatement: Promise) => ({ ...props }) => { - const LazyComponent = lazy(() => importStatement); - return ( - - {({ error, errorInfo }) => { - if (!errorInfo) { - return ( - }> - - - ); - } - - return ( -
-

Unable to find application file

-
-

Grafana has likely been updated. Please try reloading the page.

-
-
- -
-
- {error && error.toString()} -
- {errorInfo.componentStack} -
-
- ); - }} - - ); -}; diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index a5c82315741..5c52bfbdb9b 100644 --- a/public/app/routes/routes.ts +++ b/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) { diff --git a/yarn.lock b/yarn.lock index 6c96274b142..c14e3cd315a 100644 --- a/yarn.lock +++ b/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"