Widgets editor saves all the widgets instead of just the changed ones

[adamziel]

Description

When I have a lot of legacy widgets and only update one of them, clicking "Update" still sends one (batched) API request per widgets, even for the ones that were not changed at all.

saveWidgetArea has a check in place to prevent that situation, but it's not effective:

gutenberg/packages/edit-widgets/src/store/actions.js

Lines 159 to 183 in be7954e

	// Update an existing widget.
	yield dispatch(
	'core',
	'editEntityRecord',
	'root',
	'widget',
	widgetId,
	{
	...widget,
	sidebar: widgetAreaId,
	},
	{ undoIgnore: true }
	);
	const hasEdits = yield select(
	'core',
	'hasEditsForEntityRecord',
	'root',
	'widget',
	widgetId
	);
	if ( ! hasEdits ) {
	continue;
	}

This is because the editEntityRecord call effectively passes the following diff:

{
    id: '1;,
    id_base: 'text',
    instance: {
        encoded: "YToyOntzOjU6InRpdGxlIjtzOjM6IjEyMyI7czo3OiJjb250ZW50IjtzOjQ6InRlc3QiO30=",
        hash: "0ef0ad8be02953f40eaabe7dba543aa5",
        raw: {title: "123", content: "test"}
    },
    sidebar: widgetAreaId
}

which then, in editEntityRecord, is compared against the data already stored in that widget entity:

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

Lines 276 to 286 in be7954e

	// Clear edits when they are equal to their persisted counterparts
	// so that the property is not considered dirty.
	edits: Object.keys( edits ).reduce( ( acc, key ) => {
	const recordValue = record[ key ];
	const editedRecordValue = editedRecord[ key ];
	const value = mergedEdits[ key ]
	? { ...editedRecordValue, ...edits[ key ] }
	: edits[ key ];
	acc[ key ] = isEqual( recordValue, value ) ? undefined : value;
	return acc;
	}, {} ),

The problem is that for the instance field the isEqual( recordValue, value ) effectively unfolds to this:

isEqual(
    { title: "123", content: "test" },
    {
        encoded: "YToyOntzOjU6InRpdGxlIjtzOjM6IjEyMyI7czo3OiJjb250ZW50IjtzOjQ6InRlc3QiO30=",
        hash: "0ef0ad8be02953f40eaabe7dba543aa5",
        raw: {title: "123", content: "test"}
    }
)

Because the recordValue comes from the record retrieved via getRawEntityRecord where only the raw value is returned when available:

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

Lines 208 to 216 in be7954e

	// Because edits are the "raw" attribute values,
	// we return those from record selectors to make rendering,
	// comparisons, and joins with edits easier.
	accumulator[ _key ] = get(
	record[ _key ],
	'raw',
	record[ _key ]
	);
	return accumulator;

Possible solutions

A workaround could involve not passing the instance to editEntityRecord if it hasn't changed – we can detect that in saveWidgetArea.

A more appropriate solution could involve not using raw data. I am not sure why we rely on it though, any ideas @noisysocks, @kevin940726?

[noisysocks]

Because the recordValue comes from the record retrieved via getRawEntityRecord where only the raw value is returned when available:

This was the surprising bit for me while reading your description. I don't think getRawEntityRecord should do instance = instance.raw for widgets. I suspect it assumes that raw always means content.raw in a post. Can we add a guard so that it only does this for post entities?

[adamziel]

@noisysocks I don't know enough about raw to be able to answer that so cc @youknowriad and @gziolo.

Why do we even use raw here if we don't want that behavior?

[gziolo]

I don't know how getRawEntityRecord works 🤷🏻‍♂️

[noisysocks]

It's just that .raw means different things in the two different contexts.

• content.raw in a post or a page refers to HTML with block markup as opposed to content.rendered which is the HTML without block markup. This is the HTML we want to feed into the block editor. That's why getRawEntityRecord selects raw.
• instance.raw in a widget refers to the un-serialized widget attributes as opposed to instance.encoded which is the base64 encoded attributes.

getRawEntityRecord assumes that every entity with a property named raw is a post entity. That's not true.

[youknowriad]

getRawEntityRecord assumes that every entity with a property named raw is a post entity. That's not true.

Feels like this logic should move to the "entities" config array somehow (not sure how exactly)

[adamziel]

@youknowriad I proposed a solution in #34388

[adamziel]

Solved in #34388