Skip to content

Commit

Permalink
Use Select: Fix render queue. (#19286)
Browse files Browse the repository at this point in the history
* Use Select: Fix render queue issues.

* Use Select: Make `isMounted` more informative.

* Framework: Reset lockfile changes to dependencies

Co-authored-by: Andrew Duthie <andrew@andrewduthie.com>
  • Loading branch information
epiqueras and aduth committed Jan 20, 2020
1 parent ee6cc56 commit d4b7692
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 13 deletions.
8 changes: 7 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
"lodash": "^4.17.15",
"memize": "^1.0.5",
"redux": "^4.0.0",
"turbo-combine-reducers": "^1.0.2"
"turbo-combine-reducers": "^1.0.2",
"use-memo-one": "^1.1.1"
},
"publishConfig": {
"access": "public"
Expand Down
32 changes: 21 additions & 11 deletions packages/data/src/components/use-select/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
/**
* External dependencies
*/
import { useMemoOne } from 'use-memo-one';

/**
* WordPress dependencies
*/
import { createQueue } from '@wordpress/priority-queue';
import {
useLayoutEffect,
useRef,
useMemo,
useCallback,
useEffect,
useReducer,
Expand Down Expand Up @@ -78,14 +82,17 @@ export default function useSelect( _mapSelect, deps ) {
const mapSelect = useCallback( _mapSelect, deps );
const registry = useRegistry();
const isAsync = useAsyncMode();
const queueContext = useMemo( () => ( { queue: true } ), [ registry ] );
// React can sometimes clear the `useMemo` cache.
// We use the cache-stable `useMemoOne` to avoid
// losing queues.
const queueContext = useMemoOne( () => ( { queue: true } ), [ registry ] );
const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 );

const latestMapSelect = useRef();
const latestIsAsync = useRef( isAsync );
const latestMapOutput = useRef();
const latestMapOutputError = useRef();
const isMounted = useRef();
const isMountedAndNotUnsubscribing = useRef();

let mapOutput;

Expand All @@ -96,9 +103,7 @@ export default function useSelect( _mapSelect, deps ) {
mapOutput = latestMapOutput.current;
}
} catch ( error ) {
let errorMessage = `An error occurred while running 'mapSelect': ${
error.message
}`;
let errorMessage = `An error occurred while running 'mapSelect': ${ error.message }`;

if ( latestMapOutputError.current ) {
errorMessage += `\nThe error may be correlated with this previous error:\n`;
Expand All @@ -111,18 +116,23 @@ export default function useSelect( _mapSelect, deps ) {

useIsomorphicLayoutEffect( () => {
latestMapSelect.current = mapSelect;
latestMapOutput.current = mapOutput;
latestMapOutputError.current = undefined;
isMountedAndNotUnsubscribing.current = true;

// This has to run after the other ref updates
// to avoid using stale values in the flushed
// callbacks or potentially overwriting a
// changed `latestMapOutput.current`.
if ( latestIsAsync.current !== isAsync ) {
latestIsAsync.current = isAsync;
renderQueue.flush( queueContext );
}
latestMapOutput.current = mapOutput;
latestMapOutputError.current = undefined;
isMounted.current = true;
} );

useIsomorphicLayoutEffect( () => {
const onStoreChange = () => {
if ( isMounted.current ) {
if ( isMountedAndNotUnsubscribing.current ) {
try {
const newMapOutput = latestMapSelect.current(
registry.select,
Expand Down Expand Up @@ -156,7 +166,7 @@ export default function useSelect( _mapSelect, deps ) {
} );

return () => {
isMounted.current = false;
isMountedAndNotUnsubscribing.current = false;
unsubscribe();
renderQueue.flush( queueContext );
};
Expand Down

0 comments on commit d4b7692

Please sign in to comment.