From f0fc3a2d6d7b9f5b396a7619106b40c0440503db Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 15 Jan 2019 15:21:05 +0000 Subject: [PATCH] Components: Optimize withFilters, avoiding per-instance bindings (#13231) * Components: Use single hook delegator for withFilters * Component: Share component definition for withFilters instances * Components: Use slash for withFilters subgrouping * Components: Assign FilteredComponent only once renderer constructed * Component: Handle withFilters forced update through delegator Ensures that FilteredComponent definition is only computed once per animation frame debounce --- packages/components/CHANGELOG.md | 7 ++ .../src/higher-order/with-filters/index.js | 113 +++++++++++++----- 2 files changed, 88 insertions(+), 32 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index eb67ff9e79332..93591222faad9 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -1,3 +1,10 @@ +## 7.1.0 (Unreleased) + +### Improvements + +- `withFilters` has been optimized to avoid binding hook handlers for each mounted instance of the component, instead using a single centralized hook delegator. +- `withFilters` has been optimized to reuse a single shared component definition for all filtered instances of the component. + ## 7.0.5 (2019-01-03) ## 7.0.4 (2018-12-12) diff --git a/packages/components/src/higher-order/with-filters/index.js b/packages/components/src/higher-order/with-filters/index.js index 6c621c679b377..e44a41cee35af 100644 --- a/packages/components/src/higher-order/with-filters/index.js +++ b/packages/components/src/higher-order/with-filters/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { debounce, uniqueId } from 'lodash'; +import { debounce, without } from 'lodash'; /** * WordPress dependencies @@ -24,45 +24,94 @@ const ANIMATION_FRAME_PERIOD = 16; */ export default function withFilters( hookName ) { return createHigherOrderComponent( ( OriginalComponent ) => { - return class FilteredComponent extends Component { - /** @inheritdoc */ - constructor( props ) { - super( props ); - - this.onHooksUpdated = this.onHooksUpdated.bind( this ); - this.Component = applyFilters( hookName, OriginalComponent ); - this.namespace = uniqueId( 'core/with-filters/component-' ); - this.throttledForceUpdate = debounce( () => { - this.Component = applyFilters( hookName, OriginalComponent ); - this.forceUpdate(); - }, ANIMATION_FRAME_PERIOD ); - - addAction( 'hookRemoved', this.namespace, this.onHooksUpdated ); - addAction( 'hookAdded', this.namespace, this.onHooksUpdated ); + const namespace = 'core/with-filters/' + hookName; + + /** + * The component definition with current filters applied. Each instance + * reuse this shared reference as an optimization to avoid excessive + * calls to `applyFilters` when many instances exist. + * + * @type {?Component} + */ + let FilteredComponent; + + /** + * Initializes the FilteredComponent variable once, if not already + * assigned. Subsequent calls are effectively a noop. + */ + function ensureFilteredComponent() { + if ( FilteredComponent === undefined ) { + FilteredComponent = applyFilters( hookName, OriginalComponent ); } + } - /** @inheritdoc */ - componentWillUnmount() { - this.throttledForceUpdate.cancel(); - removeAction( 'hookRemoved', this.namespace ); - removeAction( 'hookAdded', this.namespace ); + class FilteredComponentRenderer extends Component { + constructor() { + super( ...arguments ); + + ensureFilteredComponent(); + } + + componentDidMount() { + FilteredComponentRenderer.instances.push( this ); + + // If there were previously no mounted instances for components + // filtered on this hook, add the hook handler. + if ( FilteredComponentRenderer.instances.length === 1 ) { + addAction( 'hookRemoved', namespace, onHooksUpdated ); + addAction( 'hookAdded', namespace, onHooksUpdated ); + } } - /** - * When a filter is added or removed for the matching hook name, the wrapped component should re-render. - * - * @param {string} updatedHookName Name of the hook that was updated. - */ - onHooksUpdated( updatedHookName ) { - if ( updatedHookName === hookName ) { - this.throttledForceUpdate(); + componentWillUnmount() { + FilteredComponentRenderer.instances = without( + FilteredComponentRenderer.instances, + this + ); + + // If this was the last of the mounted components filtered on + // this hook, remove the hook handler. + if ( FilteredComponentRenderer.instances.length === 0 ) { + removeAction( 'hookRemoved', namespace ); + removeAction( 'hookAdded', namespace ); } } - /** @inheritdoc */ render() { - return ; + return ; + } + } + + FilteredComponentRenderer.instances = []; + + /** + * Updates the FilteredComponent definition, forcing a render for each + * mounted instance. This occurs a maximum of once per animation frame. + */ + const throttledForceUpdate = debounce( () => { + // Recreate the filtered component, only after delay so that it's + // computed once, even if many filters added. + FilteredComponent = applyFilters( hookName, OriginalComponent ); + + // Force each instance to render. + FilteredComponentRenderer.instances.forEach( ( instance ) => { + instance.forceUpdate(); + } ); + }, ANIMATION_FRAME_PERIOD ); + + /** + * When a filter is added or removed for the matching hook name, each + * mounted instance should re-render with the new filters having been + * applied to the original component. + * + * @param {string} updatedHookName Name of the hook that was updated. + */ + function onHooksUpdated( updatedHookName ) { + if ( updatedHookName === hookName ) { + throttledForceUpdate(); } - }; + } + + return FilteredComponentRenderer; }, 'withFilters' ); }