Skip to content

Commit

Permalink
Use error boundaries when lazy-loading components
Browse files Browse the repository at this point in the history 
If we have an exception where we do not use an error boundary, I
added a comment explaining why.
I also standardized our usages of `lazy` and `Suspense` by
destructuring the React import.
  • Loading branch information
@jportner
jportner committed Feb 25, 2021
1 parent a21ca9a commit 032b5cb
Show file tree
Hide file tree
Showing 19 changed files with 237 additions and 116 deletions.
15 changes: 8 additions & 7 deletions ...s/public/advanced_settings/components/advanced_settings_title/advanced_settings_title.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
*/

import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner, EuiTitle } from '@elastic/eui';
import React, { useEffect, useState } from 'react';
import React, { lazy, Suspense, useEffect, useState } from 'react';

import { FormattedMessage } from '@kbn/i18n/react';
import type { Space } from 'src/plugins/spaces_oss/common';

import { getSpaceAvatarComponent } from '../../../space_avatar';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
getActiveSpace: () => Promise<Space>;
}
Expand All @@ -26,16 +31,12 @@ export const AdvancedSettingsTitle = (props: Props) => {

if (!activeSpace) return null;

const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

return (
<EuiFlexGroup gutterSize="s" responsive={false} alignItems={'center'}>
<EuiFlexItem grow={false}>
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<LazySpaceAvatar space={activeSpace} />
</React.Suspense>
</Suspense>
</EuiFlexItem>
<EuiFlexItem style={{ marginLeft: '10px' }}>
<EuiTitle size="m">
Expand Down
18 changes: 10 additions & 8 deletions ...lugins/spaces/public/copy_saved_objects_to_space/components/selectable_spaces_control.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@ import './selectable_spaces_control.scss';

import type { EuiSelectableOption } from '@elastic/eui';
import { EuiIconTip, EuiLoadingSpinner, EuiSelectable } from '@elastic/eui';
import React, { Fragment } from 'react';
import React, { lazy, Suspense } from 'react';

import { FormattedMessage } from '@kbn/i18n/react';
import type { Space } from 'src/plugins/spaces_oss/common';

import { getSpaceAvatarComponent } from '../../space_avatar';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
spaces: Space[];
selectedSpaceIds: string[];
Expand All @@ -31,9 +36,6 @@ export const SelectableSpacesControl = (props: Props) => {
return <EuiLoadingSpinner />;
}

const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);
const disabledIndicator = (
<EuiIconTip
content={
Expand Down Expand Up @@ -71,7 +73,7 @@ export const SelectableSpacesControl = (props: Props) => {
}

return (
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<EuiSelectable
options={options}
onChange={(newOptions) => updateSelectedSpaces(newOptions as SpaceOption[])}
Expand All @@ -85,13 +87,13 @@ export const SelectableSpacesControl = (props: Props) => {
>
{(list, search) => {
return (
<Fragment>
<>
{search}
{list}
</Fragment>
</>
);
}}
</EuiSelectable>
</React.Suspense>
</Suspense>
);
};
21 changes: 10 additions & 11 deletions x-pack/plugins/spaces/public/copy_saved_objects_to_space/components/space_result.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
EuiSpacer,
EuiText,
} from '@elastic/eui';
import React, { useState } from 'react';
import React, { lazy, Suspense, useState } from 'react';

import type { Space } from 'src/plugins/spaces_oss/common';

Expand All @@ -25,6 +25,11 @@ import type { ImportRetry } from '../types';
import { CopyStatusSummaryIndicator } from './copy_status_summary_indicator';
import { SpaceCopyResultDetails } from './space_result_details';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
space: Space;
summarizedCopyResult: SummarizedCopyToSpaceResult;
Expand All @@ -43,9 +48,6 @@ const getInitialDestinationMap = (objects: SummarizedCopyToSpaceResult['objects'

export const SpaceResultProcessing = (props: Pick<Props, 'space'>) => {
const { space } = props;
const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

return (
<EuiAccordion
Expand All @@ -55,9 +57,9 @@ export const SpaceResultProcessing = (props: Pick<Props, 'space'>) => {
buttonContent={
<EuiFlexGroup responsive={false}>
<EuiFlexItem grow={false}>
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<LazySpaceAvatar space={space} size="s" />
</React.Suspense>
</Suspense>
</EuiFlexItem>
<EuiFlexItem>
<EuiText>{space.name}</EuiText>
Expand Down Expand Up @@ -86,9 +88,6 @@ export const SpaceResult = (props: Props) => {
const onDestinationMapChange = (value?: Map<string, string>) => {
setDestinationMap(value || getInitialDestinationMap(objects));
};
const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

return (
<EuiAccordion
Expand All @@ -98,9 +97,9 @@ export const SpaceResult = (props: Props) => {
buttonContent={
<EuiFlexGroup responsive={false}>
<EuiFlexItem grow={false}>
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<LazySpaceAvatar space={space} size="s" />
</React.Suspense>
</Suspense>
</EuiFlexItem>
<EuiFlexItem>
<EuiText>{space.name}</EuiText>
Expand Down
56 changes: 38 additions & 18 deletions .../plugins/spaces/public/copy_saved_objects_to_space/copy_saved_objects_to_space_action.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,43 @@
* 2.0.
*/

import { EuiLoadingSpinner } from '@elastic/eui';
import React, { lazy, useMemo } from 'react';
import React, { lazy, useEffect, useState } from 'react';

import { i18n } from '@kbn/i18n';
import type { NotificationsStart, StartServicesAccessor } from 'src/core/public';
import type { SavedObjectsManagementRecord } from 'src/plugins/saved_objects_management/public';

import { SavedObjectsManagementAction } from '../../../../../src/plugins/saved_objects_management/public';
import type { PluginsStart } from '../plugin';
import { SuspenseErrorBoundary } from '../suspense_error_boundary';
import type { CopyToSpaceFlyoutProps } from './components';
import { getCopyToSpaceFlyoutComponent } from './components';

const Wrapper = (props: CopyToSpaceFlyoutProps) => {
const LazyComponent = useMemo(
() => lazy(() => getCopyToSpaceFlyoutComponent().then((component) => ({ default: component }))),
[]
);
const LazyCopyToSpaceFlyout = lazy(() =>
getCopyToSpaceFlyoutComponent().then((component) => ({ default: component }))
);

interface WrapperProps {
getStartServices: StartServicesAccessor<PluginsStart>;
props: CopyToSpaceFlyoutProps;
}

const Wrapper = ({ getStartServices, props }: WrapperProps) => {
const [notifications, setNotifications] = useState<NotificationsStart | undefined>();
useEffect(() => {
getStartServices().then(([coreStart]) => {
setNotifications(coreStart.notifications);
});
});

if (!notifications) {
return null;
}

return (
<React.Suspense fallback={<EuiLoadingSpinner />}>
<LazyComponent {...props} />
</React.Suspense>
<SuspenseErrorBoundary notifications={notifications}>
<LazyCopyToSpaceFlyout {...props} />
</SuspenseErrorBoundary>
);
};

Expand All @@ -48,7 +65,7 @@ export class CopyToSpaceSavedObjectsManagementAction extends SavedObjectsManagem
},
};

constructor() {
constructor(private getStartServices: StartServicesAccessor<PluginsStart>) {
super();
}

Expand All @@ -57,15 +74,18 @@ export class CopyToSpaceSavedObjectsManagementAction extends SavedObjectsManagem
throw new Error('No record available! `render()` was likely called before `start()`.');
}

const savedObjectTarget = {
type: this.record.type,
id: this.record.id,
namespaces: this.record.namespaces ?? [],
title: this.record.meta.title,
icon: this.record.meta.icon,
const props: CopyToSpaceFlyoutProps = {
onClose: this.onClose,
savedObjectTarget: {
type: this.record.type,
id: this.record.id,
namespaces: this.record.namespaces ?? [],
title: this.record.meta.title,
icon: this.record.meta.icon,
},
};

return <Wrapper onClose={this.onClose} savedObjectTarget={savedObjectTarget} />;
return <Wrapper getStartServices={this.getStartServices} props={props} />;
};

private onClose = () => {
Expand Down
7 changes: 5 additions & 2 deletions .../plugins/spaces/public/copy_saved_objects_to_space/copy_saved_objects_to_space_service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@
* 2.0.
*/

import type { StartServicesAccessor } from 'src/core/public';
import type { SavedObjectsManagementPluginSetup } from 'src/plugins/saved_objects_management/public';

import type { PluginsStart } from '../plugin';
import { CopyToSpaceSavedObjectsManagementAction } from './copy_saved_objects_to_space_action';

interface SetupDeps {
savedObjectsManagementSetup: SavedObjectsManagementPluginSetup;
getStartServices: StartServicesAccessor<PluginsStart>;
}

export class CopySavedObjectsToSpaceService {
public setup({ savedObjectsManagementSetup }: SetupDeps) {
const action = new CopyToSpaceSavedObjectsManagementAction();
public setup({ savedObjectsManagementSetup, getStartServices }: SetupDeps) {
const action = new CopyToSpaceSavedObjectsManagementAction(getStartServices);
savedObjectsManagementSetup.actions.register(action);
}
}
15 changes: 8 additions & 7 deletions x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
EuiTitle,
} from '@elastic/eui';
import type { ChangeEvent } from 'react';
import React, { Component, Fragment } from 'react';
import React, { Component, Fragment, lazy, Suspense } from 'react';

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
Expand All @@ -31,6 +31,11 @@ import { SectionPanel } from '../section_panel';
import { CustomizeSpaceAvatar } from './customize_space_avatar';
import { SpaceIdentifier } from './space_identifier';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
validator: SpaceValidator;
space: Partial<Space>;
Expand Down Expand Up @@ -63,10 +68,6 @@ export class CustomizeSpace extends Component<Props, State> {
initialFocus: 'input[name="spaceInitials"]',
};

const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

return (
<SectionPanel title={panelTitle} description={panelTitle}>
<EuiDescribedFormGroup
Expand Down Expand Up @@ -162,9 +163,9 @@ export class CustomizeSpace extends Component<Props, State> {
)}
onClick={this.togglePopover}
>
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<LazySpaceAvatar space={this.props.space} size="l" />
</React.Suspense>
</Suspense>
</button>
}
closePopover={this.closePopover}
Expand Down
14 changes: 8 additions & 6 deletions x-pack/plugins/spaces/public/management/spaces_grid/spaces_grid_page.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
EuiText,
EuiTitle,
} from '@elastic/eui';
import React, { Component, Fragment } from 'react';
import React, { Component, Fragment, lazy, Suspense } from 'react';

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
Expand All @@ -40,6 +40,11 @@ import type { SpacesManager } from '../../spaces_manager';
import { ConfirmDeleteModal, UnauthorizedPrompt } from '../components';
import { getEnabledFeatures } from '../lib/feature_utils';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
spacesManager: SpacesManager;
notifications: NotificationsStart;
Expand Down Expand Up @@ -259,15 +264,12 @@ export class SpacesGridPage extends Component<Props, State> {
name: '',
width: '50px',
render: (value: string, record: Space) => {
const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);
return (
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<EuiLink {...reactRouterNavigate(this.props.history, this.getEditSpacePath(record))}>
<LazySpaceAvatar space={record} size="s" />
</EuiLink>
</React.Suspense>
</Suspense>
);
},
},
Expand Down
14 changes: 8 additions & 6 deletions x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
EuiText,
} from '@elastic/eui';
import type { ReactElement } from 'react';
import React, { Component } from 'react';
import React, { Component, lazy, Suspense } from 'react';

import type { InjectedIntl } from '@kbn/i18n/react';
import { FormattedMessage, injectI18n } from '@kbn/i18n/react';
Expand All @@ -27,6 +27,11 @@ import { addSpaceIdToPath, ENTER_SPACE_PATH, SPACE_SEARCH_COUNT_THRESHOLD } from
import { getSpaceAvatarComponent } from '../../space_avatar';
import { ManageSpacesButton } from './manage_spaces_button';

// No need to wrap LazySpaceAvatar in an error boundary, because it is one of the first chunks loaded when opening Kibana.
const LazySpaceAvatar = lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);

interface Props {
id: string;
spaces: Space[];
Expand Down Expand Up @@ -187,13 +192,10 @@ class SpacesMenuUI extends Component<Props, State> {
};

private renderSpaceMenuItem = (space: Space): JSX.Element => {
const LazySpaceAvatar = React.lazy(() =>
getSpaceAvatarComponent().then((component) => ({ default: component }))
);
const icon = (
<React.Suspense fallback={<EuiLoadingSpinner />}>
<Suspense fallback={<EuiLoadingSpinner />}>
<LazySpaceAvatar space={space} size={'s'} />
</React.Suspense>
</Suspense>
);
return (
<EuiContextMenuItem
Expand Down
Loading

0 comments on commit 032b5cb

Please sign in to comment.