Core-data: do not publish outdated state to subscribers during updates (

#19752)

* Core-data: do not publish outdated state to subscribers during updates

Calling `saveEntityRecord` with an update does the following:

1. Calls `getEntityRecord` to fetch the current persisted state of the entity record
2. Calls `receiveEntityRecords` with the new up-to-date state to render the updates
3. Sends an API fetch with the update patch to persist the update
4. Calls `receiveEntityRecords` again with the new up-to-date *persisted*
state

The issue here, is that point 1 (Calling `getEntityRecord`) not only fetches
the persisted state, but it also internally calls `receiveEntityRecords` itself .
This results in the persisted outdated server state to be rendered
on the UI, causing a flickering effect, that jumps pack when point 2
takes turn.

This commit removes the call to getEntityRecord, and instead, it just
calls receiveEntityRecords with the local up-to-date state of the entity
record. This fixes the flickering issue.

* Core-data: update tests to match saveEntityRecord yeilded actions

Given `saveEntityRecord` no longer selects `getEntityRecord`,
which itself triggers a SELECT action, two SELECTs are no longer
yielded. This commit removes the expectation of these two SELECTs.

* Core-data: Introduce getEntityRecordNoResolver selector

To allow saveEntityRecord access the latest local full version
of an entity without issung an API request. This prevents
propogating outdating states to subscribers when
saveEntityRecord is called.

See: #19752 (comment)

* Address review comments at #19752:

1. Capitalize alll added comment messages
2. Alias getEntityRecord with getEntityRecordNoResolver instead of copying it
3. Use describe.each instaed of looping manually in selectors tests

docs/designers-developers/developers/data/data-core.md

@@ -202,6 +202,21 @@ _Returns_
 
 -   `?Object`: The entity record's non transient edits.
 
+<a name="getEntityRecordNoResolver" href="#getEntityRecordNoResolver">#</a> **getEntityRecordNoResolver**
+
+Returns the Entity's record object by key. Doesn't trigger a resolver nor requests the entity from the API if the entity record isn't available in the local state.
+
+_Parameters_
+
+-   _state_ `Object`: State tree
+-   _kind_ `string`: Entity kind.
+-   _name_ `string`: Entity name.
+-   _key_ `number`: Record's key
+
+_Returns_
+
+-   `?Object`: Record.
+
 <a name="getEntityRecords" href="#getEntityRecords">#</a> **getEntityRecords**
 
 Returns the Entity's records.

packages/core-data/README.md

@@ -415,6 +415,21 @@ _Returns_
 
 -   `?Object`: The entity record's non transient edits.
 
+<a name="getEntityRecordNoResolver" href="#getEntityRecordNoResolver">#</a> **getEntityRecordNoResolver**
+
+Returns the Entity's record object by key. Doesn't trigger a resolver nor requests the entity from the API if the entity record isn't available in the local state.
+
+_Parameters_
+
+-   _state_ `Object`: State tree
+-   _kind_ `string`: Entity kind.
+-   _name_ `string`: Entity name.
+-   _key_ `number`: Record's key
+
+_Returns_
+
+-   `?Object`: Record.
+
 <a name="getEntityRecords" href="#getEntityRecords">#</a> **getEntityRecords**
 
 Returns the Entity's records.

packages/core-data/src/actions.js

@@ -357,11 +357,10 @@ export function* saveEntityRecord(
 				}
 			}
 
-			// We perform an optimistic update here to clear all the edits that
-			// will be persisted so that if the server filters them, the new
-			// filtered values are always accepted.
+			// Get the full local version of the record before the update,
+			// to merge it with the edits and then propagate it to subscribers
 			persistedEntity = yield select(
-				'getEntityRecord',
+				'getEntityRecordNoResolver',
 				kind,
 				name,
 				recordId

packages/core-data/src/selectors.js

@@ -107,6 +107,20 @@ export function getEntityRecord( state, kind, name, key ) {
 	return get( state.entities.data, [ kind, name, 'queriedData', 'items', key ] );
 }
 
+/**
+ * Returns the Entity's record object by key. Doesn't trigger a resolver nor requests the entity from the API if the entity record isn't available in the local state.
+ *
+ * @param {Object} state  State tree
+ * @param {string} kind   Entity kind.
+ * @param {string} name   Entity name.
+ * @param {number} key    Record's key
+ *
+ * @return {Object?} Record.
+ */
+export function getEntityRecordNoResolver( state, kind, name, key ) {
+	return getEntityRecord( state, kind, name, key );
+}
+
 /**
  * Returns the entity's record object by key,
  * with its attributes mapped to their raw values.

packages/core-data/src/test/actions.js

@@ -41,8 +41,11 @@ describe( 'saveEntityRecord', () => {
 		expect( fulfillment.next( entities ).value.type ).toBe(
 			'SAVE_ENTITY_RECORD_START'
 		);
+
+		// Should select getEntityRecordNoResolver selector (as opposed to getEntityRecord)
+		// see https://github.com/WordPress/gutenberg/pull/19752#discussion_r368498318.
 		expect( fulfillment.next().value.type ).toBe( 'SELECT' );
-		expect( fulfillment.next().value.type ).toBe( 'SELECT' );
+		expect( fulfillment.next().value.selectorName ).toBe( 'getEntityRecordNoResolver' );
 		expect( fulfillment.next().value.type ).toBe( 'SELECT' );
 		expect( fulfillment.next().value.type ).toBe( 'RECEIVE_ITEMS' );
 		const { value: apiFetchAction } = fulfillment.next( {} );

packages/core-data/src/test/selectors.js

@@ -8,6 +8,7 @@ import deepFreeze from 'deep-freeze';
  */
 import {
 	getEntityRecord,
+	getEntityRecordNoResolver,
 	getEntityRecords,
 	getEntityRecordChangesByRecord,
 	getEntityRecordNonTransientEdits,
@@ -20,7 +21,11 @@ import {
 	getReferenceByDistinctEdits,
 } from '../selectors';
 
-describe( 'getEntityRecord', () => {
+// getEntityRecord and getEntityRecordNoResolver selectors share the same tests
+describe.each( [
+	[ getEntityRecord ],
+	[ getEntityRecordNoResolver ],
+] )( '%p', ( selector ) => {
 	it( 'should return undefined for unknown record’s key', () => {
 		const state = deepFreeze( {
 			entities: {
@@ -36,7 +41,7 @@ describe( 'getEntityRecord', () => {
 				},
 			},
 		} );
-		expect( getEntityRecord( state, 'root', 'postType', 'post' ) ).toBe( undefined );
+		expect( selector( state, 'root', 'postType', 'post' ) ).toBe( undefined );
 	} );
 
 	it( 'should return a record by key', () => {
@@ -56,7 +61,9 @@ describe( 'getEntityRecord', () => {
 				},
 			},
 		} );
-		expect( getEntityRecord( state, 'root', 'postType', 'post' ) ).toEqual( { slug: 'post' } );
+		expect( selector( state, 'root', 'postType', 'post' ) ).toEqual( {
+			slug: 'post',
+		} );
 	} );
 } );