feat: change error handling (#29)

* feat: change error handling * marginy review fixes
gravity-ui · Jul 22, 2022 · 993958a · 993958a
1 parent 8342073
commit 993958a
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 89 deletions.
diff --git a/src/components/ChartKit.tsx b/src/components/ChartKit.tsx
@@ -1,7 +1,8 @@
 import React from 'react';
 import block from 'bem-cn-lite';
-import {settings} from '../libs';
-import {getRandomCKId} from '../utils';
+import {i18n} from '../i18n';
+import {CHARTKIT_ERROR_CODE, ChartKitError, settings} from '../libs';
+import {getRandomCKId, typedMemo} from '../utils';
 import type {ChartkitType, ChartKitRef, ChartKitWidgetRef, ChartKitProps} from '../types';
 import {ErrorBoundary} from './ErrorBoundary/ErrorBoundary';
 import {Loader} from './Loader/Loader';
@@ -12,49 +13,67 @@ import './ChartKit.scss';
 
 const b = block('chartkit');
 
-const ChartKitComponent = React.forwardRef<ChartKitRef | undefined, ChartKitProps<ChartkitType>>(
-    (props, ref) => {
-        const widgetRef = React.useRef<ChartKitWidgetRef>();
-        const {id = getRandomCKId(), type, data, onLoad, onError, ...restProps} = props;
-        const lang = settings.get('lang');
-        const plugins = settings.get('plugins');
-        const plugin = plugins.find((iteratedPlugin) => iteratedPlugin.type === type);
-
-        if (!plugin) {
-            return null;
-        }
-
-        const ChartComponent = plugin.renderer;
-
-        React.useImperativeHandle(
-            ref,
-            () => ({
-                reflow(details) {
-                    if (widgetRef.current?.reflow) {
-                        widgetRef.current.reflow(details);
-                    }
-                },
-            }),
-            [],
-        );
-
-        return (
-            <ErrorBoundary onError={onError}>
-                <React.Suspense fallback={<Loader />}>
-                    <div className={b()}>
-                        <ChartComponent
-                            ref={widgetRef}
-                            id={id}
-                            lang={lang}
-                            data={data}
-                            onLoad={onLoad}
-                            {...restProps}
-                        />
-                    </div>
-                </React.Suspense>
-            </ErrorBoundary>
-        );
-    },
-);
-
-export const ChartKit = React.memo(ChartKitComponent);
+type ChartKitComponentProps<T extends ChartkitType> = Omit<ChartKitProps<T>, 'onError'> & {
+    instanceRef?: React.ForwardedRef<ChartKitRef | undefined>;
+};
+
+const ChartKitComponent = <T extends ChartkitType>(props: ChartKitComponentProps<T>) => {
+    const widgetRef = React.useRef<ChartKitWidgetRef>();
+    const {instanceRef, id = getRandomCKId(), type, data, onLoad, ...restProps} = props;
+    const lang = settings.get('lang');
+    const plugins = settings.get('plugins');
+    const plugin = plugins.find((iteratedPlugin) => iteratedPlugin.type === type);
+
+    if (!plugin) {
+        throw new ChartKitError({
+            code: CHARTKIT_ERROR_CODE.UNKNOWN_PLUGIN,
+            message: i18n('error', 'label_unknown-plugin', {type}),
+        });
+    }
+
+    const ChartComponent = plugin.renderer;
+
+    React.useImperativeHandle(
+        instanceRef,
+        () => ({
+            reflow(details) {
+                if (widgetRef.current?.reflow) {
+                    widgetRef.current.reflow(details);
+                }
+            },
+        }),
+        [],
+    );
+
+    return (
+        <React.Suspense fallback={<Loader />}>
+            <div className={b()}>
+                <ChartComponent
+                    ref={widgetRef}
+                    id={id}
+                    lang={lang}
+                    data={data}
+                    onLoad={onLoad}
+                    {...restProps}
+                />
+            </div>
+        </React.Suspense>
+    );
+};
+
+const ChartKitComponentWithErrorBoundary = React.forwardRef<
+    ChartKitRef | undefined,
+    ChartKitProps<ChartkitType>
+>((props, ref) => {
+    const {onError, ...componentProps} = props;
+
+    return (
+        <ErrorBoundary onError={onError}>
+            <ChartKitComponent instanceRef={ref} {...componentProps} />
+        </ErrorBoundary>
+    );
+}) /* https://stackoverflow.com/a/58473012 */ as <T extends ChartkitType>(
+    props: ChartKitProps<T> & {ref?: React.ForwardedRef<ChartKitRef | undefined>},
+) => ReturnType<typeof ChartKitComponent>;
+
+export const ChartKit = typedMemo(ChartKitComponentWithErrorBoundary);
diff --git a/src/components/ErrorBoundary/ErrorBoundary.tsx b/src/components/ErrorBoundary/ErrorBoundary.tsx
@@ -1,4 +1,5 @@
 import React from 'react';
+import type {ChartKitError} from '../../libs';
 import type {ChartKitOnError} from '../../types';
 import {ErrorView} from '../ErrorView/ErrorView';
 
@@ -7,7 +8,7 @@ type Props = {
 };
 
 type State = {
-    error?: Error;
+    error?: ChartKitError | Error;
 };
 
 export class ErrorBoundary extends React.Component<Props, State> {
@@ -28,8 +29,10 @@ export class ErrorBoundary extends React.Component<Props, State> {
     }
 
     render() {
-        if (this.state.error) {
-            return <ErrorView />;
+        const {error} = this.state;
+
+        if (error) {
+            return <ErrorView error={error} />;
         }
 
         return this.props.children;

diff --git a/src/components/ErrorView/ErrorView.scss b/src/components/ErrorView/ErrorView.scss
diff --git a/src/components/ErrorView/ErrorView.tsx b/src/components/ErrorView/ErrorView.tsx
@@ -1,16 +1,13 @@
 import React from 'react';
-import block from 'bem-cn-lite';
 import {i18n} from '../../i18n';
+import type {ChartKitError} from '../../libs';
 
-import './ErrorView.scss';
+type Props = {
+    error: ChartKitError | Error;
+};
 
-const b = block('chartkit-error');
+export const ErrorView = ({error}: Props) => {
+    const message = error.message || i18n('error', 'label_unknown-error');
 
-export const ErrorView = () => {
-    return (
-        <div className={b()}>
-            <div className={b('title')}>{i18n('common', 'error')}</div>
-            <div className={b('message')}>{i18n('common', 'error-unknown-extension')}</div>
-        </div>
-    );
+    return <div>{message}</div>;
 };
diff --git a/src/i18n/keysets/en.json b/src/i18n/keysets/en.json
@@ -1,8 +1,10 @@
 {
   "common": {
-    "error": "Error",
-    "error-unknown-extension": "Unknown chart type",
     "tooltip-sum": "Sum",
     "tooltip-rest": "Rest"
+  },
+  "error": {
+    "label_unknown-plugin": "Unknown plugin type \"{{type}}\"",
+    "label_unknown-error": "Unknown error"
   }
 }
diff --git a/src/i18n/keysets/ru.json b/src/i18n/keysets/ru.json
@@ -1,8 +1,10 @@
 {
   "common": {
-    "error": "Ошибка",
-    "error-unknown-extension": "Неизвестный тип чарта",
     "tooltip-sum": "Сумма",
     "tooltip-rest": "Остальные"
+  },
+  "error": {
+    "label_unknown-plugin": "Неизвестный тип плагина \"{{type}}\"",
+    "label_unknown-error": "Неизвестная ошибка"
   }
 }
diff --git a/src/libs/chartkit-error/chartkit-error.ts b/src/libs/chartkit-error/chartkit-error.ts
@@ -5,9 +5,9 @@ export type ChartKitErrorArgs = {
 };
 
 export const CHARTKIT_ERROR_CODE = {
+    NO_DATA: 'ERR.CK.NO_DATA',
     UNKNOWN: 'ERR.CK.UNKNOWN_ERROR',
     UNKNOWN_PLUGIN: 'ERR.CK.UNKNOWN_PLUGIN',
-    NO_DATA: 'ERR.CK.NO_DATA',
 };
 
 export class ChartKitError extends Error {

diff --git a/src/utils/index.ts b/src/utils/index.ts
@@ -1 +1,2 @@
 export {getRandomCKId} from './common';
+export {typedMemo} from './react';
diff --git a/src/utils/react.ts b/src/utils/react.ts
@@ -0,0 +1,5 @@
+import React from 'react';
+
+// For some reason React.memo drops the generic prop type and creates a regular union type
+// https://github.com/DefinitelyTyped/DefinitelyTyped/issues/37087#issuecomment-542793243
+export const typedMemo: <T>(component: T) => T = React.memo;
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		export {getRandomCKId} from './common';
		export {typedMemo} from './react';