Skip to content

Commit

Permalink
Refactor some components to match the react-hooks/exhaustive-deps e…
Browse files Browse the repository at this point in the history
…slint rules (#25064)
  • Loading branch information
kevin940726 committed Oct 21, 2020
1 parent 4b40677 commit 2e25f1c
Show file tree
Hide file tree
Showing 10 changed files with 464 additions and 78 deletions.
57 changes: 39 additions & 18 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ function BlockListBlock( {
[ clientId ]
);
const { removeBlock } = useDispatch( 'core/block-editor' );
const onRemove = useCallback( () => removeBlock( clientId ), [ clientId ] );
const onRemove = useCallback( () => removeBlock( clientId ), [
clientId,
removeBlock,
] );

// Handling the error state
const [ hasError, setErrorState ] = useState( false );
Expand Down Expand Up @@ -214,23 +217,41 @@ function BlockListBlock( {
);
}

const value = {
clientId,
rootClientId,
isSelected,
isFirstMultiSelected,
isLastMultiSelected,
isPartOfMultiSelection,
enableAnimation,
index,
className: wrapperClassName,
isLocked,
name,
mode,
blockTitle: blockType.title,
wrapperProps: omit( wrapperProps, [ 'data-align' ] ),
};
const memoizedValue = useMemo( () => value, Object.values( value ) );
const wrapperPropsOmitAlign = omit( wrapperProps, [ 'data-align' ] );
const memoizedValue = useMemo(
() => ( {
clientId,
rootClientId,
isSelected,
isFirstMultiSelected,
isLastMultiSelected,
isPartOfMultiSelection,
enableAnimation,
index,
className: wrapperClassName,
isLocked,
name,
mode,
blockTitle: blockType.title,
wrapperProps: wrapperPropsOmitAlign,
} ),
[
clientId,
rootClientId,
isSelected,
isFirstMultiSelected,
isLastMultiSelected,
isPartOfMultiSelection,
enableAnimation,
index,
wrapperClassName,
isLocked,
name,
mode,
blockType.title,
wrapperPropsOmitAlign,
]
);

let block;

Expand Down
37 changes: 37 additions & 0 deletions packages/compose/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ _Parameters_

- _args_ `...any`: Arguments passed to Lodash's `debounce`.

<a name="useDidMount" href="#useDidMount">#</a> **useDidMount**

A drop-in replacement of the hook version of `componentDidMount`.
Like `useEffect` but only called once when the component is mounted.
This hook is only used for backward-compatibility reason. Consider using `useEffect` wherever possible.

_Parameters_

- _effect_ `Function`: The effect callback passed to `useEffect`.

<a name="useInstanceId" href="#useInstanceId">#</a> **useInstanceId**

Provides a unique instance ID.
Expand All @@ -176,6 +186,22 @@ _Parameters_
- _callback_ `Function`: Shortcut callback.
- _options_ `WPKeyboardShortcutConfig`: Shortcut options.

<a name="useLazyRef" href="#useLazyRef">#</a> **useLazyRef**

Like `useRef` but only run the initializer once.

_Parameters_

- _initializer_ `Function`: A function to return the ref object.

_Returns_

- `MutableRefObject`: The returned ref object.

_Type Definition_

- _MutableRefObject_ (unknown type)

<a name="useMediaQuery" href="#useMediaQuery">#</a> **useMediaQuery**

Runs a media query and returns its value when it changes.
Expand Down Expand Up @@ -233,6 +259,17 @@ _Returns_

- `Array`: An array of {Element} `resizeListener` and {?Object} `sizes` with properties `width` and `height`

<a name="useShallowCompareEffect" href="#useShallowCompareEffect">#</a> **useShallowCompareEffect**

Like `useEffect` but call the effect when the dependencies are not shallowly equal.
Useful when the size of the dependency array might change during re-renders.
This hook is only used for backward-compatibility reason. Consider using `useEffect` wherever possible.

_Parameters_

- _effect_ `Function`: The effect callback passed to `useEffect`.
- _deps_ `Array`: The dependency array that is compared against shallowly.

<a name="useViewportMatch" href="#useViewportMatch">#</a> **useViewportMatch**

Returns true if the viewport matches the given query, or false otherwise.
Expand Down
21 changes: 21 additions & 0 deletions packages/compose/src/hooks/use-did-mount/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* WordPress dependencies
*/
import { useLayoutEffect, useRef } from '@wordpress/element';

/**
* A drop-in replacement of the hook version of `componentDidMount`.
* Like `useEffect` but only called once when the component is mounted.
* This hook is only used for backward-compatibility reason. Consider using `useEffect` wherever possible.
*
* @param {Function} effect The effect callback passed to `useEffect`.
*/
function useDidMount( effect ) {
const effectRef = useRef( effect );
effectRef.current = effect;

// `useLayoutEffect` because that's closer to how the `componentDidMount` works.
useLayoutEffect( () => effectRef.current(), [] );
}

export default useDidMount;
91 changes: 91 additions & 0 deletions packages/compose/src/hooks/use-did-mount/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* External dependencies
*/
import { render } from '@testing-library/react';

/**
* WordPress dependencies
*/
import React, { Component, useEffect } from '@wordpress/element';

/**
* Internal dependencies
*/
import useDidMount from '../';

describe( 'useDidMount', () => {
it( 'should call the effect when did mount', () => {
const mountEffect = jest.fn();

function TestComponent() {
useDidMount( mountEffect );
return null;
}

render( <TestComponent /> );

expect( mountEffect ).toHaveBeenCalledTimes( 1 );
} );

it( 'should call the cleanup function when unmount', () => {
const unmountCallback = jest.fn();

function TestComponent() {
useDidMount( () => unmountCallback );
return null;
}

const { unmount } = render( <TestComponent /> );

expect( unmountCallback ).not.toHaveBeenCalled();

unmount();

expect( unmountCallback ).toHaveBeenCalledTimes( 1 );
} );

it( 'should match the calling order of componentDidMount', async () => {
const mountEffectCallback = jest.fn();
const effectCallback = jest.fn();

const didMountCallback = jest.fn(
() =>
new Promise( ( resolve ) => {
expect( mountEffectCallback ).toHaveBeenCalled();
expect( effectCallback ).not.toHaveBeenCalled();

resolve();
} )
);

let promise;

class DidMount extends Component {
componentDidMount() {
promise = didMountCallback();
}
render() {
return null;
}
}

function Hook() {
useDidMount( mountEffectCallback );
useEffect( effectCallback );
return null;
}

function TestComponent() {
return (
<>
<Hook />
<DidMount />
</>
);
}

render( <TestComponent /> );

await promise;
} );
} );
26 changes: 26 additions & 0 deletions packages/compose/src/hooks/use-lazy-ref/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* WordPress dependencies
*/
import { useRef } from '@wordpress/element';

const INITIAL_TAG = Symbol( 'INITIAL_TAG' );

/**
* Like `useRef` but only run the initializer once.
*
* @typedef {import('@types/react').MutableRefObject} MutableRefObject
*
* @param {Function} initializer A function to return the ref object.
* @return {MutableRefObject} The returned ref object.
*/
function useLazyRef( initializer ) {
const ref = useRef( INITIAL_TAG );

if ( ref.current === INITIAL_TAG ) {
ref.current = initializer();
}

return ref;
}

export default useLazyRef;
82 changes: 82 additions & 0 deletions packages/compose/src/hooks/use-lazy-ref/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* WordPress dependencies
*/
import React, { useReducer } from '@wordpress/element';

/**
* External dependencies
*/
import { render, act } from '@testing-library/react';

/**
* Internal dependencies
*/
import useLazyRef from '..';

describe( 'useLazyRef', () => {
it( 'should lazily initialize the initializer only once', () => {
const initializer = jest.fn( () => 87 );
let result;
let forceUpdate = () => {};

function TestComponent() {
const ref = useLazyRef( initializer );

forceUpdate = useReducer( ( c ) => c + 1, 0 )[ 1 ];

result = ref.current;

return null;
}

render( <TestComponent /> );

expect( initializer ).toHaveBeenCalledTimes( 1 );
expect( result ).toBe( 87 );

act( () => {
forceUpdate();
} );

expect( initializer ).toHaveBeenCalledTimes( 1 );
expect( result ).toBe( 87 );
} );

it( 'should not accept falsy values', () => {
const initializer = jest.fn( () => 87 );
let result;
let ref = { current: null };
let forceUpdate = () => {};

function TestComponent() {
ref = useLazyRef( initializer );

forceUpdate = useReducer( ( c ) => c + 1, 0 )[ 1 ];

result = ref.current;

return null;
}

render( <TestComponent /> );

expect( initializer ).toHaveBeenCalledTimes( 1 );
expect( result ).toBe( 87 );

ref.current = undefined;
act( () => {
forceUpdate();
} );

expect( initializer ).toHaveBeenCalledTimes( 1 );
expect( result ).toBe( undefined );

ref.current = null;
act( () => {
forceUpdate();
} );

expect( initializer ).toHaveBeenCalledTimes( 1 );
expect( result ).toBe( null );
} );
} );
26 changes: 26 additions & 0 deletions packages/compose/src/hooks/use-shallow-compare-effect/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* WordPress dependencies
*/
import { useEffect, useRef } from '@wordpress/element';
import isShallowEqual from '@wordpress/is-shallow-equal';

/**
* Like `useEffect` but call the effect when the dependencies are not shallowly equal.
* Useful when the size of the dependency array might change during re-renders.
* This hook is only used for backward-compatibility reason. Consider using `useEffect` wherever possible.
*
* @param {Function} effect The effect callback passed to `useEffect`.
* @param {Array} deps The dependency array that is compared against shallowly.
*/
function useShallowCompareEffect( effect, deps ) {
const ref = useRef();

if ( ! isShallowEqual( ref.current, deps ) ) {
ref.current = deps;
}

// eslint-disable-next-line react-hooks/exhaustive-deps
useEffect( effect, [ ref.current ] );
}

export default useShallowCompareEffect;
Loading

0 comments on commit 2e25f1c

Please sign in to comment.