[6.7] Fix: recreate handlers and reset completed state on expression change (#33900) (#34483)

Backports the following commits to 6.7:
 - Fix: recreate handlers and reset completed state on expression change  (#33900)

x-pack/plugins/canvas/public/components/element_content/element_content.js

@@ -24,6 +24,7 @@ const branches = [
   branch(({ renderable, state }) => {
     return !state || !renderable;
   }, renderComponent(({ backgroundColor }) => <Loading backgroundColor={backgroundColor} />)),
+
   // renderable is available, but no matching element is found, render invalid
   branch(({ renderable, renderFunction }) => {
     return renderable && getType(renderable) !== 'render' && !renderFunction;

x-pack/plugins/canvas/public/components/element_share_container/element_share_container.js

@@ -20,30 +20,21 @@ export class ElementShareContainer extends React.PureComponent {
   };
 
   componentDidMount() {
-    const { functionName, onComplete } = this.props;
+    const { onComplete } = this.props;
-    const isDevelopment = process.env.NODE_ENV !== 'production';
 
     // check that the done event is called within a certain time
-    if (isDevelopment) {
+    this.createDoneChecker();
-      const timeout = 15; // timeout, in seconds
-      this.timeout = setTimeout(() => {
-        // TODO: show this message in a proper notification
-        console.warn(
-          `done handler not called in render function after ${timeout} seconds: ${functionName}`
-        );
-      }, timeout * 1000);
-    }
 
     // dispatches a custom DOM event on the container when the element is complete
     onComplete(() => {
-      clearTimeout(this.timeout);
+      this.clearDoneChecker();
       if (!this.sharedItemRef) {
         return;
       } // without this, crazy fast forward/backward paging leads to an error
       const ev = new CustomEvent('renderComplete');
       this.sharedItemRef.dispatchEvent(ev);
 
-      // if the element is finished before reporting is listening for then
+      // if the element is finished before reporting is listening for the
       // renderComplete event, the report never completes. to get around that
       // issue, track the completed state locally and set the
       // [data-render-complete] value accordingly.
@@ -53,10 +44,42 @@ export class ElementShareContainer extends React.PureComponent {
     });
   }
 
-  componentWillUnmount() {
+  getSnapshotBeforeUpdate(prevProps) {
-    clearTimeout(this.timeout);
+    return { functionName: prevProps.functionName };
   }
 
+  componentDidUpdate(prevProps, prevState, snapshot) {
+    // if function name changed, clear and recreate done checker
+    if (snapshot.functionName !== this.props.functionName) {
+      this.clearDoneChecker();
+      this.createDoneChecker();
+    }
+  }
+
+  componentWillUnmount() {
+    this.clearDoneChecker();
+  }
+
+  createDoneChecker = () => {
+    const isDevelopment = process.env.NODE_ENV !== 'production';
+    if (!isDevelopment) {
+      return;
+    }
+
+    const { functionName } = this.props;
+    const timeout = 15; // timeout, in seconds
+    this.timeout = setTimeout(() => {
+      // TODO: show this message in a proper notification
+      console.warn(
+        `done handler not called in render function after ${timeout} seconds: ${functionName}`
+      );
+    }, timeout * 1000);
+  };
+
+  clearDoneChecker = () => {
+    clearTimeout(this.timeout);
+  };
+
   render() {
     // NOTE: the data-shared-item and data-render-complete attributes are used for reporting
     return (

x-pack/plugins/canvas/public/components/element_wrapper/element_wrapper.js

@@ -9,30 +9,23 @@ import PropTypes from 'prop-types';
 import { Positionable } from '../positionable';
 import { ElementContent } from '../element_content';
 
-export class ElementWrapper extends React.PureComponent {
+export const ElementWrapper = props => {
-  static propTypes = {
+  const { renderable, transformMatrix, width, height, state, handlers } = props;
-    renderable: PropTypes.object,
-    transformMatrix: PropTypes.arrayOf(PropTypes.number).isRequired,
-    width: PropTypes.number.isRequired,
-    height: PropTypes.number.isRequired,
-    state: PropTypes.string,
-    createHandlers: PropTypes.func.isRequired,
-  };
-
-  constructor(props) {
-    super(props);
-    this._handlers = props.createHandlers(props.selectedPage);
-  }
-
-  _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._handlers} />
+      <ElementContent renderable={renderable} state={state} handlers={handlers} />
     </Positionable>
   );
-  }
+};
-}
+
+ElementWrapper.propTypes = {
+  // positionable props (from element object)
+  transformMatrix: PropTypes.arrayOf(PropTypes.number).isRequired,
+  width: PropTypes.number.isRequired,
+  height: PropTypes.number.isRequired,
+  // ElementContent pass-through props
+  renderable: PropTypes.object,
+  state: PropTypes.string,
+  handlers: PropTypes.object.isRequired,
+};

x-pack/plugins/canvas/public/components/element_wrapper/index.js

@@ -5,52 +5,73 @@
  */
 
 import PropTypes from 'prop-types';
-import { connect } from 'react-redux';
+import { connectAdvanced } from 'react-redux';
-import { compose, shouldUpdate } from 'recompose';
+import { compose, withPropsOnChange, mapProps } from 'recompose';
 import isEqual from 'react-fast-compare';
 import { getResolvedArgs, getSelectedPage } from '../../state/selectors/workpad';
-import { getState, getValue, getError } from '../../lib/resolved_arg';
+import { getState, getValue } from '../../lib/resolved_arg';
 import { ElementWrapper as Component } from './element_wrapper';
 import { createHandlers as createHandlersWithDispatch } from './lib/handlers';
 
-const mapStateToProps = (state, { element }) => ({
+function selectorFactory(dispatch) {
-  resolvedArg: getResolvedArgs(state, element.id, 'expressionRenderable'),
+  let result = {};
-  selectedPage: getSelectedPage(state),
+  const createHandlers = createHandlersWithDispatch(dispatch);
-});
 
-const mapDispatchToProps = (dispatch, { element }) => ({
+  return (nextState, nextOwnProps) => {
-  createHandlers: pageId => createHandlersWithDispatch(element, pageId, dispatch),
+    const { element, ...restOwnProps } = nextOwnProps;
-});
+    const { transformMatrix, width, height } = element;
 
-const mergeProps = (stateProps, dispatchProps, ownProps) => {
+    const resolvedArg = getResolvedArgs(nextState, element.id, 'expressionRenderable');
-  const { resolvedArg, selectedPage } = stateProps;
+    const selectedPage = getSelectedPage(nextState);
-  const { element, restProps } = ownProps;
-  const { id, transformMatrix, width, height } = element;
 
-  return {
+    // build interim props object
+    const nextResult = {
+      ...restOwnProps,
+      // state and state-derived props
       selectedPage,
-    ...restProps, // pass through unused props
+      state: getState(resolvedArg),
-    id, //pass through useful parts of the element object
+      renderable: getValue(resolvedArg),
+      // pass along the handlers creation function
+      createHandlers,
+      // required parts of the element object
       transformMatrix,
       width,
       height,
-    state: getState(resolvedArg),
+      // pass along only the useful parts of the element object
-    error: getError(resolvedArg),
+      // so handlers object can be created
-    renderable: getValue(resolvedArg),
+      element: {
-    createHandlers: dispatchProps.createHandlers,
+        id: element.id,
-  };
+        filter: element.filter,
+        expression: element.expression,
+      },
     };
 
+    // update props only if something actually changed
+    if (!isEqual(result, nextResult)) {
+      result = nextResult;
+    }
+
+    return result;
+  };
+}
+
 export const ElementWrapper = compose(
-  connect(
+  connectAdvanced(selectorFactory),
-    mapStateToProps,
+  withPropsOnChange(
-    mapDispatchToProps,
+    (props, nextProps) => !isEqual(props.element, nextProps.element),
-    mergeProps,
+    props => {
-    {
+      const { element, createHandlers } = props;
-      areOwnPropsEqual: isEqual,
+      const handlers = createHandlers(element, props.selectedPage);
+      // this removes element and createHandlers from passed props
+      return { handlers };
     }
   ),
-  shouldUpdate((props, nextProps) => !isEqual(props, nextProps))
+  mapProps(props => {
+    // remove elements and createHandlers from props passed to component
+    // eslint-disable-next-line no-unused-vars
+    const { element, createHandlers, selectedPage, ...restProps } = props;
+    return restProps;
+  })
 )(Component);
 
 ElementWrapper.propTypes = {
@@ -59,5 +80,9 @@ ElementWrapper.propTypes = {
     transformMatrix: PropTypes.arrayOf(PropTypes.number).isRequired,
     width: PropTypes.number.isRequired,
     height: PropTypes.number.isRequired,
+    // sometimes we get a shape, which lacks an expression
+    // so element properties can not be marked as required
+    expression: PropTypes.string,
+    filter: PropTypes.string,
   }).isRequired,
 };

x-pack/plugins/canvas/public/components/element_wrapper/lib/handlers.js

@@ -4,12 +4,21 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { isEqual } from 'lodash';
 import { setFilter } from '../../../state/actions/elements';
 
-export function createHandlers(element, pageId, dispatch) {
+export const createHandlers = dispatch => {
   let isComplete = false;
+  let oldElement;
   let completeFn = () => {};
 
+  return (element, pageId) => {
+    // reset isComplete when element changes
+    if (!isEqual(oldElement, element)) {
+      isComplete = false;
+      oldElement = element;
+    }
+
     return {
       setFilter(text) {
         dispatch(setFilter(text, element.id, pageId, true));
@@ -24,11 +33,14 @@ export function createHandlers(element, pageId, dispatch) {
       },
 
       done() {
+        // don't emit if the element is already done
         if (isComplete) {
           return;
-      } // don't emit if the element is already done
+        }
+
         isComplete = true;
         completeFn();
       },
     };
-}
+  };
+};

x-pack/plugins/canvas/public/components/workpad_page/index.js

@@ -121,7 +121,7 @@ export const WorkpadPage = compose(
         }
       }
       // instead of just combining `element` with `shape`, we make property transfer explicit
-      return element ? { ...shape, filter: element.filter } : shape;
+      return element ? { ...shape, filter: element.filter, expression: element.expression } : shape;
     });
     return {
       elements,