Core Data: The "cacheKey" improvements

[Mamaduka]

Sort _fields and include keys values

Both keys are normalized using the getNormalizedCommaSeparable function, but they aren't sorted. It results in having a different cache key if the order of values is changed, and triggering a new REST API request, even though we're requesting the same data.

Example:

// Do the initial request.
wp.data.select('core').getEntityRecords('taxonomy', 'post_tag', { _fields: 'id,name' });

// This triggers another request, but can use previously cached values.
wp.data.select('core').getEntityRecords('taxonomy', 'post_tag', { _fields: 'name,id' });

Update "hashing" method

Currently, we're using addQueryArgs to generate the query cache key "hash." While this method works just fine, it produces extra characters when encoding commas and ampersands.

I looked at few popular query libraries, and most of them are using JSON.stringify() to create a query cache key. So maybe we should also adopt this method.

• React Query example.
• React Toolkit Query example.

/cc @adamziel , @youknowriad, @jsnajdr

[jsnajdr]

JSON.stringify is not going to help with _fields: 'name,id'. The 'name,id' value is a string, and no generic library can understand its internal structure and the fact that the order doesn't matter for _fields. I can imagine a very similar _sort: 'name,id' option where the order matters a lot.

We'll have to treat _fields specially to fix this.

[Mamaduka]

Thanks for the feedback, @jsnajdr.

We're already converting comma-separated values into an array and then back to string for the stableKey. So we can sort them after array conversion.

See:

gutenberg/packages/core-data/src/queried-data/get-query-parts.js

Lines 74 to 83 in 1d93095

	parts.fields = getNormalizedCommaSeparable( value );
	// Make sure to normalize value for `stableKey`
	value = parts.fields.join();
	}
	// Two requests with different include values cannot have same results.
	if ( key === 'include' ) {
	parts.include = getNormalizedCommaSeparable( value ).map(
	Number
	);

I only mentioned JSON.stringify as an alternative to the current cache key generator.

// Example cache key using `addQueryArgs`
?include=0%2C1%2C2%2C3%2C4&_fields=id%2Cname

vs

// Same key using `JSON.stringify`
{"include":"0,1,2,3,4","_fields":"id,name"}

[adamziel]

There are two levels to consider here

Level 1 is @wordpress/data resolution cache. It is entity-agnostic and uses the serialize resolver arguments as keys:

gutenberg/packages/data/src/redux-store/metadata/utils.ts

Lines 48 to 59 in fb3d84b

	export function selectorArgsToStateKey( args: unknown[] | null | undefined ) {
	if ( args === undefined || args === null ) {
	return [];
	}
	const len = args.length;
	let idx = len;
	while ( idx > 0 && args[ idx - 1 ] === undefined ) {
	idx--;
	}
	return idx === len ? args : args.slice( 0, idx );
	}

I don't think it should know about the core-data-specific selectors, so we're left with layer 2: The getEntityRecord resolver in @wordpress/core-data:

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

Lines 132 to 134 in fb3d84b

	export const getEntityRecords = ( kind, name, query = {} ) => async ( {
	dispatch,
	} ) => {

It would have to compile the full query in

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

Lines 161 to 164 in fb3d84b

	const path = addQueryArgs( entityConfig.baseURL, {
	...entityConfig.baseURLParams,
	...query,
	} );

and e.g. always sort the _fields query arg so that this fragment returns before the HTTP request:

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

Lines 97 to 105 in fb3d84b

	query = { ...query, include: [ key ] };
	// The resolution cache won't consider query as reusable based on the
	// fields, so it's tested here, prior to initiating the REST request,
	// and without causing `getEntityRecords` resolution to occur.
	const hasRecords = select.hasEntityRecords( kind, name, query );
	if ( hasRecords ) {
	return;
	}

The same would be needed in getEntityRecords, only it is more complex there because there is no code path to short-circuit if there's a cache hit:

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

Lines 161 to 166 in fb3d84b

	const path = addQueryArgs( entityConfig.baseURL, {
	...entityConfig.baseURLParams,
	...query,
	} );
	let records = Object.values( await apiFetch( { path } ) );

[Mamaduka]

Thanks for the feedback, @adamziel.

I agree that "Level 1" doesn't need to know about the core-data specifics, but I would also expect any two selectors with the same arguments to return the same results.

@jsnajdr did some work to improve this (#38914, #38945).

[adamziel]

I would also expect any two selectors with the same arguments to return the same results.

I assume you meant "two selector calls", not "two selectors"? I guess it's a matter of how we define the same arguments. It would surely be nice if a selector could say "this is a string" and "this is an http query". Perhaps if it made its own resolution cache key? I'm thinking of something like getEntityRecords.resolutionCacheKey = function(kind, name, query) { }

[Mamaduka]

Closing this issue as it doesn't seem to be a priority at the moment.