useSelect: only invalidate on subscribe if store changed (#57108)

packages/core-data/src/hooks/test/use-query-select.js

@@ -50,11 +50,9 @@ describe( 'useQuerySelect', () => {
 				<TestComponent keyName="foo" />
 			</RegistryProvider>
 		);
-		// 2 times expected
-		// - 1 for initial mount
-		// - 1 for after mount before subscription set.
-		expect( selectSpy ).toHaveBeenCalledTimes( 2 );
-		expect( TestComponent ).toHaveBeenCalledTimes( 2 );
+
+		expect( selectSpy ).toHaveBeenCalledTimes( 1 );
+		expect( TestComponent ).toHaveBeenCalledTimes( 1 );
 
 		// ensure expected state was rendered
 		expect( screen.getByText( 'bar' ) ).toBeInTheDocument();
@@ -81,10 +79,9 @@ describe( 'useQuerySelect', () => {
 		);
 
 		// ensure the selectors were properly memoized
-		expect( selectors ).toHaveLength( 4 );
+		expect( selectors ).toHaveLength( 2 );
 		expect( selectors[ 0 ] ).toHaveProperty( 'testSelector' );
 		expect( selectors[ 0 ] ).toBe( selectors[ 1 ] );
-		expect( selectors[ 1 ] ).toBe( selectors[ 2 ] );
 
 		// Re-render
 		render(
@@ -94,9 +91,10 @@ describe( 'useQuerySelect', () => {
 		);
 
 		// ensure we still got the memoized results after re-rendering
-		expect( selectors ).toHaveLength( 8 );
-		expect( selectors[ 3 ] ).toHaveProperty( 'testSelector' );
-		expect( selectors[ 5 ] ).toBe( selectors[ 6 ] );
+		expect( selectors ).toHaveLength( 4 );
+		expect( selectors[ 2 ] ).toHaveProperty( 'testSelector' );
+		expect( selectors[ 1 ] ).toBe( selectors[ 2 ] );
+		expect( selectors[ 2 ] ).toBe( selectors[ 3 ] );
 	} );
 
 	it( 'returns the expected "response" details – no resolvers and arguments', () => {

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

@@ -44,6 +44,14 @@ function Store( registry, suspense ) {
 	let lastIsAsync;
 	let subscriber;
 	let didWarnUnstableReference;
+	const storeStatesOnMount = new Map();
+
+	function getStoreState( name ) {
+		// If there's no store property (custom generic store), return an empty
+		// object. When comparing the state, the empty objects will cause the
+		// equality check to fail, setting `lastMapResultValid` to false.
+		return registry.stores[ name ]?.store?.getState?.() ?? {};
+	}
 
 	const createSubscriber = ( stores ) => {
 		// The set of stores the `subscribe` function is supposed to subscribe to. Here it is
@@ -56,12 +64,24 @@ function Store( registry, suspense ) {
 		const activeSubscriptions = new Set();
 
 		function subscribe( listener ) {
-			// Invalidate the value right after subscription was created. React will
-			// call `getValue` after subscribing, to detect store updates that happened
-			// in the interval between the `getValue` call during render and creating
-			// the subscription, which is slightly delayed. We need to ensure that this
-			// second `getValue` call will compute a fresh value.
-			lastMapResultValid = false;
+			// Maybe invalidate the value right after subscription was created.
+			// React will call `getValue` after subscribing, to detect store
+			// updates that happened in the interval between the `getValue` call
+			// during render and creating the subscription, which is slightly
+			// delayed. We need to ensure that this second `getValue` call will
+			// compute a fresh value only if any of the store states have
+			// changed in the meantime.
+			if ( lastMapResultValid ) {
+				for ( const name of activeStores ) {
+					if (
+						storeStatesOnMount.get( name ) !== getStoreState( name )
+					) {
+						lastMapResultValid = false;
+					}
+				}
+			}
+
+			storeStatesOnMount.clear();
 
 			const onStoreChange = () => {
 				// Invalidate the value on store update, so that a fresh value is computed.
@@ -149,6 +169,9 @@ function Store( registry, suspense ) {
 			}
 
 			if ( ! subscriber ) {
+				for ( const name of listeningStores.current ) {
+					storeStatesOnMount.set( name, getStoreState( name ) );
+				}
 				subscriber = createSubscriber( listeningStores.current );
 			} else {
 				subscriber.updateStores( listeningStores.current );