Skip to content

Commit

Permalink
perf: code splitting in ContentPreview (#737)
Browse files Browse the repository at this point in the history
  • Loading branch information
DanDeMicco authored Jan 31, 2019
1 parent 25c3868 commit 02ef9f4
Show file tree
Hide file tree
Showing 32 changed files with 749 additions and 157 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ render(
| --- | --- | --- | --- |
| *tbd* | - | - | - |

# Code Splitting
[Code splitting](https://webpack.js.org/guides/code-splitting/) is currently supported for the `ContentPreview` and `ContentSidebar` UI Elements. In order to use an Element with code splitting, you will need to build the Element with webpack by importing the Element from the `es` folder in our npm package.

# Questions
If you have any questions, please visit our [developer forum](https://community.box.com/t5/Box-Developer-Forum/bd-p/DeveloperForum) or contact us via one of our [available support channels](https://community.box.com/t5/Community/ct-p/English).
Expand Down
2 changes: 2 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = {
'@babel/preset-flow',
],
plugins: [
'@babel/plugin-syntax-dynamic-import',
'@babel/plugin-transform-flow-strip-types',
'@babel/plugin-transform-object-assign',
'@babel/plugin-proposal-class-properties',
Expand Down Expand Up @@ -58,6 +59,7 @@ module.exports = {
test: {
plugins: [
'@babel/plugin-transform-modules-commonjs',
'dynamic-import-node', // https://github.com/facebook/jest/issues/5920
[
'react-intl',
{
Expand Down
16 changes: 16 additions & 0 deletions conf/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ function getConfig(isReactExternalized) {
}
}

if (isRelease) {
// For release builds, disable code splitting. https://webpack.js.org/api/module-methods/#magic-comments
config.module.rules = [
{
test: /\.js$/,
loader: 'string-replace-loader',
options: {
search: 'webpackMode: "lazy"',
replace: 'webpackMode: "eager"',
flags: 'g',
},
},
...config.module.rules,
];
}

if (isRelease && language === 'en-US') {
config.plugins.push(
new BundleAnalyzerPlugin({
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
"@babel/core": "^7.0.0",
"@babel/plugin-proposal-class-properties": "^7.0.0",
"@babel/plugin-proposal-object-rest-spread": "^7.0.0",
"@babel/plugin-syntax-dynamic-import": "^7.2.0",
"@babel/plugin-transform-flow-strip-types": "^7.0.0",
"@babel/plugin-transform-object-assign": "^7.0.0",
"@babel/polyfill": "^7.0.0",
Expand All @@ -142,6 +143,7 @@
"babel-eslint": "^10.0.1",
"babel-jest": "^23.6.0",
"babel-loader": "^8.0.0",
"babel-plugin-dynamic-import-node": "^2.2.0",
"babel-plugin-flow-react-proptypes": "^24.1.2",
"babel-plugin-module-resolver": "^3.1.1",
"babel-plugin-react-intl": "^2.4.0",
Expand Down Expand Up @@ -225,6 +227,7 @@
"scroll-into-view-if-needed": "^1.5.0",
"semantic-release": "^15.12.0",
"sinon": "^2.3.7",
"string-replace-loader": "^2.1.1",
"style-loader": "^0.23.0",
"stylelint": "^9.6.0",
"stylelint-config-standard": "^18.2.0",
Expand Down
58 changes: 58 additions & 0 deletions src/elements/common/async-load/AsyncLoad.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// @flow
import * as React from 'react';
import { retryNumOfTimes } from 'utils/function';

type Props = {
loader: () => Promise<any>,
fallback?: React.Element<any>,
errorComponent?: React.ComponentType<any>,
};

type State = {
error: ?Error,
};

const DEFAULT_NUM_TIMES = 3;
const DEFAULT_INITIAL_DELAY = 500;
const DEFAULT_BACKOFF_FACTOR = 2;

const NullComponent = () => null;

const AsyncLoad = (asyncProps: Props = {}) => {
const asyncLoadWithRetry = () =>
retryNumOfTimes(
(successCallback, errorCallback) => asyncProps.loader().then(successCallback, errorCallback),
DEFAULT_NUM_TIMES,
DEFAULT_INITIAL_DELAY,
DEFAULT_BACKOFF_FACTOR,
);
const LazyComponent = React.lazy(asyncLoadWithRetry);

return class extends React.Component<Object, State> {
state = {
error: null,
};

static getDerivedStateFromError(error: Error) {
return {
error,
};
}

render() {
const { fallback = null, errorComponent: ErrorComponent = NullComponent } = asyncProps;
const { error } = this.state;
if (error) {
return <ErrorComponent error={error} />;
}

return (
<React.Suspense fallback={fallback}>
<LazyComponent {...this.props} />
</React.Suspense>
);
}
};
};

export default AsyncLoad;
51 changes: 51 additions & 0 deletions src/elements/common/async-load/__tests__/AsyncLoad-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import * as React from 'react';
import { shallow } from 'enzyme';
import { retryNumOfTimes } from 'utils/function';
import AsyncLoad from '../AsyncLoad';

jest.mock('utils/function', () => ({
retryNumOfTimes: jest.fn(),
}));

describe('elements/common/async-load/AsyncLoad', () => {
let defaultProps;

const getAsyncComponent = (props = defaultProps) => AsyncLoad(props);

beforeEach(() => {
defaultProps = {
loader: jest.fn(),
};
});

test('should return a react component', () => {
const AsyncComponent = getAsyncComponent();
expect(AsyncComponent.prototype).toBeInstanceOf(React.Component);
});

test('should load the lazy component', () => {
const AsyncComponent = getAsyncComponent();
const wrapper = shallow(<AsyncComponent foo="bar" />);
expect(wrapper).toMatchSnapshot();
});

test('should render the error component if there is an error', () => {
const errorComponent = () => <div>ERROR!!</div>;
const AsyncComponent = getAsyncComponent({
...defaultProps,
errorComponent,
});

const wrapper = shallow(<AsyncComponent />);
wrapper.setState({
error: new Error('foo'),
});

expect(wrapper).toMatchSnapshot();
});

test('should not invoke the loader until component mounted', () => {
const AsyncComponent = getAsyncComponent();
expect(retryNumOfTimes).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`elements/common/async-load/AsyncLoad should load the lazy component 1`] = `
<React.Suspense
fallback={null}
>
<React.Lazy
foo="bar"
/>
</React.Suspense>
`;

exports[`elements/common/async-load/AsyncLoad should render the error component if there is an error 1`] = `
<errorComponent
error={[Error: foo]}
/>
`;
2 changes: 2 additions & 0 deletions src/elements/common/async-load/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// @flow
export { default } from './AsyncLoad';
7 changes: 6 additions & 1 deletion src/elements/content-preview/ContentPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import Measure from 'react-measure';
import { decode } from 'box-react-ui/lib/utils/keys';
import makeResponsive from 'elements/common/makeResponsive';
import Internationalize from 'elements/common/Internationalize';
import AsyncLoad from 'elements/common/async-load';
import TokenService from 'utils/TokenService';
import { isInputElement, focus } from 'utils/dom';
import { getTypedFileId } from 'utils/file';
Expand Down Expand Up @@ -140,6 +141,10 @@ const MARK_NAME_JS_READY = `${ORIGIN_CONTENT_PREVIEW}_${EVENT_JS_READY}`;

mark(MARK_NAME_JS_READY);

const LoadableSidebar = AsyncLoad({
loader: () => import(/* webpackMode: "lazy", webpackChunkName: "content-sidebar" */ '../content-sidebar'),
});

class ContentPreview extends PureComponent<Props, State> {
id: string;

Expand Down Expand Up @@ -1116,7 +1121,7 @@ class ContentPreview extends PureComponent<Props, State> {
/>
</div>
{file && (
<ContentSidebar
<LoadableSidebar
{...contentSidebarProps}
isLarge={isLarge}
apiHost={apiHost}
Expand Down
8 changes: 6 additions & 2 deletions src/elements/content-preview/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import IconDrawAnnotationMode from 'box-react-ui/lib/icons/annotations/IconDrawA
import IconPointAnnotation from 'box-react-ui/lib/icons/annotations/IconPointAnnotation';
import IconPrint from 'box-react-ui/lib/icons/general/IconPrint';
import IconDownload from 'box-react-ui/lib/icons/general/IconDownloadSolid';
import AsyncLoad from 'elements/common/async-load';
import messages from 'elements/common/messages';
import { getIcon } from 'elements/common/item/iconCellRenderer';
import { COLOR_999 } from '../../constants';
import ContentOpenWith from '../content-open-with/ContentOpenWith';
import './Header.scss';

type Props = {
Expand All @@ -29,6 +29,10 @@ type Props = {
token: ?string,
} & InjectIntlProvidedProps;

const LoadableContentOpenWith = AsyncLoad({
loader: () => import(/* webpackMode: "lazy", webpackChunkName: "content-open-with" */ '../content-open-with'),
});

const Header = ({
canAnnotate,
canDownload,
Expand Down Expand Up @@ -58,7 +62,7 @@ const Header = ({
</div>
<div className="bcpr-btns">
{shouldRenderOpenWith && (
<ContentOpenWith
<LoadableContentOpenWith
className="bcpr-bcow-btn"
fileId={id}
token={token}
Expand Down
7 changes: 5 additions & 2 deletions src/elements/content-sidebar/ActivitySidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import { EVENT_JS_READY } from 'elements/common/logger/constants';
import ActivityFeed from './activity-feed';
import SidebarContent from './SidebarContent';
import AddTaskButton from './AddTaskButton';
import { DEFAULT_COLLAB_DEBOUNCE, ORIGIN_ACTIVITY_SIDEBAR } from '../../constants';
import SidebarUtils from './SidebarUtils';
import { DEFAULT_COLLAB_DEBOUNCE, ORIGIN_ACTIVITY_SIDEBAR, SIDEBAR_VIEW_ACTIVITY } from '../../constants';
import './ActivitySidebar.scss';

type ExternalProps = {
Expand Down Expand Up @@ -72,6 +73,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {

constructor(props: Props) {
super(props);
// eslint-disable-next-line react/prop-types
const { logger } = this.props;
logger.onReadyMetric({
endMarkName: MARK_NAME_JS_READY,
Expand Down Expand Up @@ -331,6 +333,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
console.error(error);
/* eslint-enable no-console */

// eslint-disable-next-line react/prop-types
this.props.onError(error, code, contextInfo);
};

Expand Down Expand Up @@ -476,7 +479,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {

return (
<SidebarContent
title={<FormattedMessage {...messages.sidebarActivityTitle} />}
title={SidebarUtils.getTitleForView(SIDEBAR_VIEW_ACTIVITY)}
actions={<FeatureFlag feature="activityFeed.tasks.createButton" enabled={this.renderAddTaskButton} />}
>
<ActivityFeed
Expand Down
10 changes: 8 additions & 2 deletions src/elements/content-sidebar/DetailsSidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ import SidebarContent from './SidebarContent';
import SidebarVersions from './SidebarVersions';
import SidebarNotices from './SidebarNotices';
import SidebarFileProperties from './SidebarFileProperties';
import { HTTP_STATUS_CODE_FORBIDDEN, ORIGIN_DETAILS_SIDEBAR, IS_ERROR_DISPLAYED } from '../../constants';
import SidebarUtils from './SidebarUtils';
import {
HTTP_STATUS_CODE_FORBIDDEN,
ORIGIN_DETAILS_SIDEBAR,
IS_ERROR_DISPLAYED,
SIDEBAR_VIEW_DETAILS,
} from '../../constants';
import './DetailsSidebar.scss';

type ExternalProps = {
Expand Down Expand Up @@ -440,7 +446,7 @@ class DetailsSidebar extends React.PureComponent<Props, State> {
}

return (
<SidebarContent title={<FormattedMessage {...messages.sidebarDetailsTitle} />}>
<SidebarContent title={SidebarUtils.getTitleForView(SIDEBAR_VIEW_DETAILS)}>
{hasNotices && (
<div className="bcs-details-content">{hasNotices && <SidebarNotices file={file} />}</div>
)}
Expand Down
4 changes: 3 additions & 1 deletion src/elements/content-sidebar/MetadataSidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import { withLogger } from 'elements/common/logger';
import { mark } from 'utils/performance';
import { EVENT_JS_READY } from 'elements/common/logger/constants';
import SidebarContent from './SidebarContent';
import SidebarUtils from './SidebarUtils';
import {
FIELD_IS_EXTERNALLY_OWNED,
FIELD_PERMISSIONS,
FIELD_PERMISSIONS_CAN_UPLOAD,
IS_ERROR_DISPLAYED,
ORIGIN_METADATA_SIDEBAR,
SIDEBAR_VIEW_METADATA,
} from '../../constants';
import './MetadataSidebar.scss';

Expand Down Expand Up @@ -379,7 +381,7 @@ class MetadataSidebar extends React.PureComponent<Props, State> {

return (
<SidebarContent
title={<FormattedMessage {...messages.sidebarMetadataTitle} />}
title={SidebarUtils.getTitleForView(SIDEBAR_VIEW_METADATA)}
actions={
showTemplateDropdown ? (
<TemplateDropdown
Expand Down
Loading

0 comments on commit 02ef9f4

Please sign in to comment.