Skip to content

Commit

Permalink
withNotices: Memoize the noticeOperations object (#29582)
Browse files Browse the repository at this point in the history
The refactor of withNotices in #28676 introduced a bug where the
functions within `noticeOperations` were changing on every render. That
meant that if they were present in the dependency array of a useEffect,
then the effect would trigger causing an infinite loop.

This was partially fixed with #29491, but there are other instances
where the noticeOperations object itself is being listed as a
dependency, and so the problem persists.

As suggested by @stokesman
#29491 (comment)
this change memoizes the whole object.
  • Loading branch information
pablinos committed Mar 5, 2021
1 parent 330e980 commit 9dc9615
Showing 1 changed file with 44 additions and 46 deletions.
90 changes: 44 additions & 46 deletions packages/components/src/higher-order/with-notices/index.js
Expand Up @@ -6,7 +6,7 @@ import { v4 as uuid } from 'uuid';
/**
* WordPress dependencies
*/
import { forwardRef, useState, useCallback } from '@wordpress/element';
import { forwardRef, useState, useMemo } from '@wordpress/element';
import { createHigherOrderComponent } from '@wordpress/compose';

/**
Expand All @@ -25,55 +25,53 @@ export default createHigherOrderComponent( ( OriginalComponent ) => {
function Component( props, ref ) {
const [ noticeList, setNoticeList ] = useState( [] );

/**
* Function passed down as a prop that adds a new notice.
*
* @param {Object} notice Notice to add.
*/
const createNotice = useCallback( ( notice ) => {
const noticeToAdd = notice.id ? notice : { ...notice, id: uuid() };
setNoticeList( ( current ) => [ ...current, noticeToAdd ] );
}, [] );
const noticeOperations = useMemo( () => {
/**
* Function passed down as a prop that adds a new notice.
*
* @param {Object} notice Notice to add.
*/
const createNotice = ( notice ) => {
const noticeToAdd = notice.id
? notice
: { ...notice, id: uuid() };
setNoticeList( ( current ) => [ ...current, noticeToAdd ] );
};

/**
* Function passed as a prop that adds a new error notice.
*
* @param {string} msg Error message of the notice.
*/
const createErrorNotice = useCallback(
( msg ) => {
createNotice( {
status: 'error',
content: msg,
} );
},
[ createNotice ]
);
return {
createNotice,

/**
* Removes a notice by id.
*
* @param {string} id Id of the notice to remove.
*/
const removeNotice = useCallback( ( id ) => {
setNoticeList( ( current ) =>
current.filter( ( notice ) => notice.id !== id )
);
}, [] );
/**
* Function passed as a prop that adds a new error notice.
*
* @param {string} msg Error message of the notice.
*/
createErrorNotice: ( msg ) => {
createNotice( {
status: 'error',
content: msg,
} );
},

/**
* Removes all notices
*/
const removeAllNotices = useCallback( () => {
setNoticeList( [] );
}, [] );
/**
* Removes a notice by id.
*
* @param {string} id Id of the notice to remove.
*/
removeNotice: ( id ) => {
setNoticeList( ( current ) =>
current.filter( ( notice ) => notice.id !== id )
);
},

const noticeOperations = {
createNotice,
createErrorNotice,
removeNotice,
removeAllNotices,
};
/**
* Removes all notices
*/
removeAllNotices: () => {
setNoticeList( [] );
},
};
}, [] );

const propsOut = {
...props,
Expand Down

0 comments on commit 9dc9615

Please sign in to comment.