Generated state keys for selector resolution do not account for varying types of recordKey

[getdave]

Calling getEntityRecord on a populated Redux cache (can) result in unnecessary network requests.

This is dependent on whether a consumer passes the 3rd recordKey argument as a String or an Integer.

Expected behaviour: if a record already exists in state, a call to select that record by ID should not result in a network request to retrieve that record.

Actual behaviour: if a record already exists in state, a getEntityRecord call to select that record by ID using a string-based ID results in an additional network request.

Replication

You can replicate as follows:

• Create some Navigation Menus on your website using the Navigation block. Be sure to save/publish them.
• Create a new Post
• Open dev tools network tab and then hit Esc to also open the Console tab below it.
• In console type wp.data.select('core').getEntityRecords('postType', 'wp_navigation'). You should see null and then running it again you should see results. This has populated the core/data state with entities of that type.
• Note down the ids of those records.
• At this point those records are in state and thus any calls to select the individual records should not result in a network request because they are already in state and getEntityRecords resolves the getEntityRecord selectors.
• Choose the ID of one of your records and enter the following into the console making sure that you pass the 3rd argument as a string and not a int value - wp.data.select('core').getEntityRecord('postType', 'wp_navigation','{YOUR_RECORD_ID}')
• See a network request triggered to fetch a record which is already in state!
• Now take a different ID and enter the following but this time pass the 3rd argument as an integer - wp.data.select('core').getEntityRecord('postType', 'wp_navigation',{YOUR_RECORD_ID})
• See no network request is dispatched.
• Do the same query again but pass the same ID as a string and see a network request will be dispatched.

Screencapture

Screen.Capture.on.2023-06-29.at.10-15-18.mp4

Explanation

@wordpress/data has a mechanism whereby the next resolution state of a selector is determined by setting a status prop on a value in a (special kind of) Map object.

This map is is "keyed" by the selector's args as an array (remember Map can have many different types of keys compared to plain objects).

For example a call to getEntityRecords('postType', 'wp_navigation') would result in a "key" array for a given record that might look like this - ['postType', 'wp_navigation', 379)].

When it comes time to determine whether a given resolver is "resolved", we check the same Map for a key which matches the given selector's args.

gutenberg/packages/data/src/redux-store/metadata/selectors.js

Lines 22 to 29 in 3b7902e

	export function getResolutionState( state, selectorName, args ) {
	const map = state[ selectorName ];
	if ( ! map ) {
	return;
	}
	return map.get( selectorArgsToStateKey( args ) );
	}

However the bug occurs when you pass different types as the 3rd argument to getEntityRecord().

The reason this happens is that the args being set in the EquivalentKeyMap cache have different types for the 3rd (recordKey) argument. This is because consumers often call getEntityRecords interchangeably, passing the 3rd (recordKey) argument as either a number of a string (note it's also due to the way getEntityRecords resolves the getEntityRecord selector - see below).

This means that the call to

return map.get( selectorArgsToStateKey( args ) ); 

...tries to get a value using the key ['postType', 'wp_navigation', '379')] but the key is actually using the integer form ['postType', 'wp_navigation', 379)] so no match occurs.

This causes the selector to not be resolved and thus the network request is dispatched. This is wrong as the resolver should in fact be resolved.

It's worth noting that the reason a integer sends up in the state is due to how we resolve the getEntityRecord (singular) selector within the getEntityRecords (plural) resolver:

gutenberg/packages/core-data/src/resolvers.js

Lines 203 to 206 in 3b7902e

	const key = entityConfig.key || DEFAULT_ENTITY_KEY;
	const resolutionsArgs = records
	.filter( ( record ) => record[ key ] )
	.map( ( record ) => [ kind, name, record[ key ] ] );

The record[ key ] part will always result in a integer because the value of the id property of the REST API response is always a int and not a string.

Solution

I have a fix whereby I patched the selectorArgsToStateKey to convert any "numeric" strings in the args array into their integer equivalents.

This ensures that however getEntityRecord is called the cache always sets/gets using the same type.

However it might be a bit drastic and I'm not sure of any side effects.

[getdave]

I discussed this away from Github with @jsnajdr and he suggested we could fix this directly in getEntityRecord(s). I'm going to take a look at this.