From 01be7ac89b97b76c490d57a15c16466657240770 Mon Sep 17 00:00:00 2001 From: Anton Timmermans Date: Wed, 21 Nov 2018 11:02:20 +0100 Subject: [PATCH] Fix RichText rerendering when it shouldn't (#12161) * Fix RichText rerendering when it shouldn't The prepareEditableTree and onChangeEditableValue function stacks would have a new reference on every render. This prevents that by memoized the stack based on the previous stack and the newly added function. * Rename emptyArray to EMPTY_ARRAY * Add memize as a dependency to annotations package --- package-lock.json | 1 + packages/annotations/package.json | 1 + packages/annotations/src/format/annotation.js | 55 ++++++++++++++----- packages/annotations/src/store/selectors.js | 17 +++++- .../rich-text/src/register-format-type.js | 35 +++++++++--- 5 files changed, 84 insertions(+), 25 deletions(-) diff --git a/package-lock.json b/package-lock.json index e5d3df1c2322bf..90395bff8c7c94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2283,6 +2283,7 @@ "@wordpress/i18n": "file:packages/i18n", "@wordpress/rich-text": "file:packages/rich-text", "lodash": "^4.17.10", + "memize": "^1.0.5", "rememo": "^3.0.0", "uuid": "^3.3.2" } diff --git a/packages/annotations/package.json b/packages/annotations/package.json index ec6f312c18f1de..6a177913bbc094 100644 --- a/packages/annotations/package.json +++ b/packages/annotations/package.json @@ -26,6 +26,7 @@ "@wordpress/i18n": "file:../i18n", "@wordpress/rich-text": "file:../rich-text", "lodash": "^4.17.10", + "memize": "^1.0.5", "rememo": "^3.0.0", "uuid": "^3.3.2" }, diff --git a/packages/annotations/src/format/annotation.js b/packages/annotations/src/format/annotation.js index a2f6f2973c7268..e72786bad0e3bc 100644 --- a/packages/annotations/src/format/annotation.js +++ b/packages/annotations/src/format/annotation.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import memize from 'memize'; + /** * WordPress dependencies */ @@ -115,6 +120,40 @@ function updateAnnotationsWithPositions( annotations, positions, { removeAnnotat } ); } +/** + * Create prepareEditableTree memoized based on the annotation props. + * + * @param {Object} The props with annotations in them. + * + * @return {Function} The prepareEditableTree. + */ +const createPrepareEditableTree = memize( ( props ) => { + const { annotations } = props; + + return ( formats, text ) => { + if ( annotations.length === 0 ) { + return formats; + } + + let record = { formats, text }; + record = applyAnnotations( record, annotations ); + return record.formats; + }; +} ); + +/** + * Returns the annotations as a props object. Memoized to prevent re-renders. + * + * @param {Array} The annotations to put in the object. + * + * @return {Object} The annotations props object. + */ +const getAnnotationObject = memize( ( annotations ) => { + return { + annotations, + }; +} ); + export const annotation = { name: FORMAT_NAME, title: __( 'Annotation' ), @@ -128,21 +167,9 @@ export const annotation = { return null; }, __experimentalGetPropsForEditableTreePreparation( select, { richTextIdentifier, blockClientId } ) { - return { - annotations: select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ), - }; - }, - __experimentalCreatePrepareEditableTree( { annotations } ) { - return ( formats, text ) => { - if ( annotations.length === 0 ) { - return formats; - } - - let record = { formats, text }; - record = applyAnnotations( record, annotations ); - return record.formats; - }; + return getAnnotationObject( select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ) ); }, + __experimentalCreatePrepareEditableTree: createPrepareEditableTree, __experimentalGetPropsForEditableTreeChangeHandler( dispatch ) { return { removeAnnotation: dispatch( STORE_KEY ).__experimentalRemoveAnnotation, diff --git a/packages/annotations/src/store/selectors.js b/packages/annotations/src/store/selectors.js index ca6fcb64796d5f..a39a315c92f907 100644 --- a/packages/annotations/src/store/selectors.js +++ b/packages/annotations/src/store/selectors.js @@ -4,6 +4,17 @@ import createSelector from 'rememo'; import { get, flatMap } from 'lodash'; +/** + * Shared reference to an empty array for cases where it is important to avoid + * returning a new array reference on every invocation, as in a connected or + * other pure component which performs `shouldComponentUpdate` check on props. + * This should be used as a last resort, since the normalized data should be + * maintained by the reducer result in state. + * + * @type {Array} + */ +const EMPTY_ARRAY = []; + /** * Returns the annotations for a specific client ID. * @@ -19,12 +30,12 @@ export const __experimentalGetAnnotationsForBlock = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, [] ), + get( state, blockClientId, EMPTY_ARRAY ), ] ); export const __experimentalGetAllAnnotationsForBlock = function( state, blockClientId ) { - return get( state, blockClientId, [] ); + return get( state, blockClientId, EMPTY_ARRAY ); }; /** @@ -54,7 +65,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, [] ), + get( state, blockClientId, EMPTY_ARRAY ), ] ); diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js index 8b61fc3a4308b1..262812a22b248f 100644 --- a/packages/rich-text/src/register-format-type.js +++ b/packages/rich-text/src/register-format-type.js @@ -2,6 +2,7 @@ * External dependencies */ import { mapKeys } from 'lodash'; +import memize from 'memize'; /** * WordPress dependencies @@ -10,6 +11,17 @@ import { select, dispatch, withSelect, withDispatch } from '@wordpress/data'; import { addFilter } from '@wordpress/hooks'; import { compose } from '@wordpress/compose'; +/** + * Shared reference to an empty array for cases where it is important to avoid + * returning a new array reference on every invocation, as in a connected or + * other pure component which performs `shouldComponentUpdate` check on props. + * This should be used as a last resort, since the normalized data should be + * maintained by the reducer result in state. + * + * @type {Array} + */ +const EMPTY_ARRAY = []; + /** * Registers a new format provided a unique name and an object defining its * behavior. @@ -119,6 +131,13 @@ export function registerFormatType( name, settings ) { dispatch( 'core/rich-text' ).addFormatTypes( settings ); + const getFunctionStackMemoized = memize( ( previousStack = EMPTY_ARRAY, newFunction ) => { + return [ + ...previousStack, + newFunction, + ]; + } ); + if ( settings.__experimentalGetPropsForEditableTreePreparation ) { @@ -133,13 +152,13 @@ export function registerFormatType( name, settings ) { const additionalProps = {}; if ( settings.__experimentalCreatePrepareEditableTree ) { - additionalProps.prepareEditableTree = [ - ...( props.prepareEditableTree || [] ), + additionalProps.prepareEditableTree = getFunctionStackMemoized( + props.prepareEditableTree, settings.__experimentalCreatePrepareEditableTree( props[ `format_${ name }` ], { richTextIdentifier: props.identifier, blockClientId: props.clientId, - } ), - ]; + } ) + ); } if ( settings.__experimentalCreateOnChangeEditableValue ) { @@ -155,16 +174,16 @@ export function registerFormatType( name, settings ) { return accumulator; }, {} ); - additionalProps.onChangeEditableValue = [ - ...( props.onChangeEditableValue || [] ), + additionalProps.onChangeEditableValue = getFunctionStackMemoized( + props.onChangeEditableValue, settings.__experimentalCreateOnChangeEditableValue( { ...props[ `format_${ name }` ], ...dispatchProps, }, { richTextIdentifier: props.identifier, blockClientId: props.clientId, - } ), - ]; + } ) + ); } return