Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill unnecessary re-renders in ElementWrapper #31734

Merged
merged 4 commits into from
Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,19 @@ export class ElementWrapper extends React.PureComponent {
createHandlers: PropTypes.func.isRequired,
};

state = {
handlers: null,
};

componentDidMount() {
// create handlers when component mounts, so it only creates one instance
const { createHandlers } = this.props;
// eslint-disable-next-line react/no-did-mount-set-state
this.setState({ handlers: createHandlers() });
constructor(props) {
super(props);
this._handlers = props.createHandlers(props.selectedPage);
}

render() {
// wait until the handlers have been created
if (!this.state.handlers) {
return null;
}
_handlers = null;

render() {
const { renderable, transformMatrix, width, height, state } = this.props;

return (
<Positionable transformMatrix={transformMatrix} width={width} height={height}>
<ElementContent renderable={renderable} state={state} handlers={this.state.handlers} />
<ElementContent renderable={renderable} state={state} handlers={this._handlers} />
</Positionable>
);
}
Expand Down
24 changes: 16 additions & 8 deletions x-pack/plugins/canvas/public/components/element_wrapper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@

import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { getResolvedArgs, getSelectedPage, isWriteable } from '../../state/selectors/workpad';
import { compose, shouldUpdate } from 'recompose';
import isEqual from 'react-fast-compare';
import { getResolvedArgs, getSelectedPage } from '../../state/selectors/workpad';
import { getState, getValue, getError } from '../../lib/resolved_arg';
import { ElementWrapper as Component } from './element_wrapper';
import { createHandlers as createHandlersWithDispatch } from './lib/handlers';

const mapStateToProps = (state, { element }) => ({
isWriteable: isWriteable(state),
resolvedArg: getResolvedArgs(state, element.id, 'expressionRenderable'),
selectedPage: getSelectedPage(state),
});

const mapDispatchToProps = (dispatch, { element }) => ({
createHandlers: pageId => () => createHandlersWithDispatch(element, pageId, dispatch),
createHandlers: pageId => createHandlersWithDispatch(element, pageId, dispatch),
});

const mergeProps = (stateProps, dispatchProps, ownProps) => {
Expand All @@ -27,6 +28,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
const { id, transformMatrix, width, height } = element;

return {
selectedPage,
...restProps, // pass through unused props
id, //pass through useful parts of the element object
transformMatrix,
Expand All @@ -35,14 +37,20 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
state: getState(resolvedArg),
error: getError(resolvedArg),
renderable: getValue(resolvedArg),
createHandlers: dispatchProps.createHandlers(selectedPage),
createHandlers: dispatchProps.createHandlers,
};
};

export const ElementWrapper = connect(
mapStateToProps,
mapDispatchToProps,
mergeProps
export const ElementWrapper = compose(
connect(
mapStateToProps,
mapDispatchToProps,
mergeProps,
{
areOwnPropsEqual: isEqual,
}
),
shouldUpdate((props, nextProps) => !isEqual(props, nextProps))
)(Component);

ElementWrapper.propTypes = {
Expand Down