getEntityRecord issues a HTTP request when there's a cached result

[adamziel]

Describe the bug

Consider the following function:

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

Lines 54 to 64 in be01420

	export function* getEntityRecord( kind, name, key = '' ) {
	const entities = yield getKindEntities( kind );
	const entity = find( entities, { kind, name } );
	if ( ! entity ) {
	return;
	}
	const record = yield apiFetch( {
	path: `${ entity.baseURL }/${ key }?context=edit`,
	} );
	yield receiveEntityRecords( kind, name, record );
	}

I would expect it to short-circuit if the entity is found, but it does the opposite which results in extra http queries.

For example, calling:

getEntityRecords("root", "widgetArea");
// ... wait for resolution ...
getEntityRecord("root", "widgetArea", "sidebar-1");

Will issue two http queries even if the entity with id sidebar-1 is already available.

CC @youknowriad

[youknowriad]

I wonder if this is solved with #19498 which is close to land.

[adamziel]

I wonder if this is solved with #19498 which is close to land.

It seems like it will! Let's confirm once it lands and then hopefully close this issue.

Also, in reply to #24290 (comment):

this change is problematic for me. entity will always be defined aside on the very first call so getEntityRecord won't do anything for all calls except the first one?

I may be missing something, but it would make sense to me to use the entity we already have (until the cache is invalidated) instead of re-requesting a new one each time. I don't get how it's short-circuiting when the entity is not found - shouldn't that be exactly when the resolution needs to happen?.

[youknowriad]

I may be missing something, but it would make sense to me to use the entity we already have (until the cache is invalidated) instead of re-requesting a new one each time. I don't get how it's short-circuiting when the entity is not found - shouldn't that be exactly when the resolution needs to happen?.

I think you're confusing "Entity" and "Entity Record". "Entity" is just the config of the entity, we're resolving the record at this point.

[adamziel]

Ah I see, it all makes sense now! Awesome, so #19498 will solve this.

[kevin940726]

Since #19498 has been merged, this issue should be resolved now :)