useSelect: prevent nested component update after unmount (#40677)

* useSelect: prevent nested component update after unmount

* Update store name

packages/data/src/components/use-select/index.js

@@ -191,6 +191,8 @@ export default function useSelect( mapSelect, deps ) {
 		}
 	} );
 
+	const isMounted = useRef( false );
+
 	useIsomorphicLayoutEffect( () => {
 		if ( ! hasMappingFunction ) {
 			return;
@@ -211,6 +213,10 @@ export default function useSelect( mapSelect, deps ) {
 		};
 
 		const onChange = () => {
+			if ( ! isMounted.current ) {
+				return;
+			}
+
 			if ( latestIsAsync.current ) {
 				renderQueue.add( queueContext, onStoreChange );
 			} else {
@@ -226,10 +232,13 @@ export default function useSelect( mapSelect, deps ) {
 			registry.__unstableSubscribeStore( storeName, onChange )
 		);
 
+		isMounted.current = true;
+
 		return () => {
 			// The return value of the subscribe function could be undefined if the store is a custom generic store.
 			unsubscribers.forEach( ( unsubscribe ) => unsubscribe?.() );
 			renderQueue.cancel( queueContext );
+			isMounted.current = false;
 		};
 		// If you're tempted to eliminate the spread dependencies below don't do it!
 		// We're passing these in from the calling function and want to make sure we're

packages/data/src/components/use-select/test/index.js

@@ -118,6 +118,71 @@ describe( 'useSelect', () => {
 		expect( rendered.getByRole( 'status' ) ).toHaveTextContent( 'bar' );
 	} );
 
+	it( 'avoid calling nested listener after unmounted', async () => {
+		registry.registerStore( 'toggler', {
+			reducer: ( state = false, action ) =>
+				action.type === 'TOGGLE' ? ! state : state,
+			actions: {
+				toggle: () => ( { type: 'TOGGLE' } ),
+			},
+			selectors: {
+				get: ( state ) => state,
+			},
+		} );
+
+		const mapSelect = ( select ) => select( 'toggler' ).get();
+
+		const mapSelectChild = jest.fn( mapSelect );
+		function Child() {
+			const show = useSelect( mapSelectChild, [] );
+			return show ? 'yes' : 'no';
+		}
+
+		const mapSelectParent = jest.fn( mapSelect );
+		function Parent() {
+			const show = useSelect( mapSelectParent, [] );
+			return show ? <Child /> : 'none';
+		}
+
+		const rendered = render(
+			<RegistryProvider value={ registry }>
+				<Parent />
+			</RegistryProvider>
+		);
+
+		// Initial render renders only parent and subscribes the parent to store.
+		expect( rendered.getByText( 'none' ) ).toBeInTheDocument();
+		expect( mapSelectParent ).toHaveBeenCalledTimes( 2 );
+		expect( mapSelectChild ).toHaveBeenCalledTimes( 0 );
+
+		// act() does batched updates internally, i.e., any scheduled setStates or effects
+		// will be executed only after the dispatch finishes. But we want to opt out of
+		// batched updates here. We want all the setStates to be done synchronously, as the
+		// store listeners are called. The async/await code is a trick to do it: do the
+		// dispatch in a different event loop tick, where the batched updates are no longer active.
+		await act( async () => {
+			await Promise.resolve();
+			registry.dispatch( 'toggler' ).toggle();
+		} );
+
+		// Child was rendered and subscribed to the store, as the _second_ subscription.
+		expect( rendered.getByText( 'yes' ) ).toBeInTheDocument();
+		expect( mapSelectParent ).toHaveBeenCalledTimes( 3 );
+		expect( mapSelectChild ).toHaveBeenCalledTimes( 2 );
+
+		await act( async () => {
+			await Promise.resolve();
+			registry.dispatch( 'toggler' ).toggle();
+		} );
+
+		// Check that child was unmounted without any extra state update being performed on it.
+		// I.e., `mapSelectChild` was never called again, and no "state update on an unmounted
+		// component" warning was triggered.
+		expect( rendered.getByText( 'none' ) ).toBeInTheDocument();
+		expect( mapSelectParent ).toHaveBeenCalledTimes( 4 );
+		expect( mapSelectChild ).toHaveBeenCalledTimes( 2 );
+	} );
+
 	describe( 'rerenders as expected with various mapSelect return types', () => {
 		const getComponent = ( mapSelectSpy ) => () => {
 			const data = useSelect( mapSelectSpy, [] );