Interactivity API: Update data-wp-bind directive logic

packages/e2e-tests/plugins/interactive-blocks/directive-bind/render.php

@@ -56,4 +56,44 @@
 	>
 		Some Text
 	</p>
+
+	<?php
+	$hydration_cases = array(
+		'false'       => '{ "value": false }',
+		'true'        => '{ "value": true }',
+		'null'        => '{ "value": null }',
+		'undef'       => '{ "__any": "any" }',
+		'emptyString' => '{ "value": "" }',
+		'anyString'   => '{ "value": "any" }',
+		'number'      => '{ "value": 10 }',
+	);
+	?>
+
+	<?php foreach ( $hydration_cases as $type => $context ) : ?>
+	<div
+		data-testid='hydrating <?php echo $type; ?>'
+		data-wp-context='<?php echo $context; ?>'
+	>
+		<img
+			alt="Red dot"
+			data-testid="image"
+			data-wp-bind--width="context.value"
+			src="data:image/png;base64, iVBORw0KGgoAAAANSUhEUgAAAAUA
+			AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
+			9TXL0Y4OHwAAAABJRU5ErkJggg=="
+		>
+		<input
+			type="text"
+			data-testid="input"
+			data-wp-bind--name="context.value"
+			data-wp-bind--value="context.value"
+			data-wp-bind--disabled="context.value"
+			data-wp-bind--aria-disabled="context.value"
+		>
+		<button
+			data-testid="toggle value"
+			data-wp-on--click="actions.toggleValue"
+		>Toggle</button>
+	</div>
+	<?php endforeach; ?>
 </div>

packages/e2e-tests/plugins/interactive-blocks/directive-bind/view.js

@@ -18,6 +18,17 @@
 				state.show = ! state.show;
 				state.width += foo.bar;
 			},
+			toggleValue: ( { context } ) => {
+				const previousValue = ( 'previousValue' in context )
+					? context.previousValue
+					// Any string works here; we just want to toggle the value
+					// to ensure Preact renders the same we are hydrating in the
+					// first place.
+					: 'tacocat';
+
+				context.previousValue = context.value;
+				context.value = previousValue;
+			}
 		},
 	} );
 } )( window );

packages/interactivity/CHANGELOG.md

@@ -7,6 +7,7 @@
 -   Support keys using `data-wp-key`. ([#53844](https://github.com/WordPress/gutenberg/pull/53844))
 -   Merge new server-side rendered context on client-side navigation. ([#53853](https://github.com/WordPress/gutenberg/pull/53853))
 -   Support region-based client-side navigation. ([#53733](https://github.com/WordPress/gutenberg/pull/53733))
+-   Improve `data-wp-bind` hydration to match Preact's logic. ([#54003](https://github.com/WordPress/gutenberg/pull/54003))
 
 ## 2.1.0 (2023-08-16)
 

packages/interactivity/src/directives.js

@@ -222,19 +222,46 @@ export default () => {
 					// on the hydration, so we have to do it manually. It doesn't need
 					// deps because it only needs to do it the first time.
 					useEffect( () => {
+						const el = element.ref.current;
+
+						// We set the value directly to the corresponding
+						// HTMLElement instance property excluding the following
+						// special cases.
+						// We follow Preact's logic: https://github.com/preactjs/preact/blob/ea49f7a0f9d1ff2c98c0bdd66aa0cbc583055246/src/diff/props.js#L110-L129
+						if (
+							attribute !== 'width' &&
+							attribute !== 'height' &&
+							attribute !== 'href' &&
+							attribute !== 'list' &&
+							attribute !== 'form' &&
+							// Default value in browsers is `-1` and an empty string is
+							// cast to `0` instead
+							attribute !== 'tabIndex' &&
+							attribute !== 'download' &&
+							attribute !== 'rowSpan' &&
+							attribute !== 'colSpan' &&
+							attribute in el
+						) {
+							try {
+								el[ attribute ] =
+									result === null || result === undefined
+										? ''
+										: result;
+								return;
+							} catch ( err ) {}
+						}
 						// aria- and data- attributes have no boolean representation.
 						// A `false` value is different from the attribute not being
 						// present, so we can't remove it.
 						// We follow Preact's logic: https://github.com/preactjs/preact/blob/ea49f7a0f9d1ff2c98c0bdd66aa0cbc583055246/src/diff/props.js#L131C24-L136
-						if ( result === false && attribute[ 4 ] !== '-' ) {
-							element.ref.current.removeAttribute( attribute );
+						if (
+							result !== null &&
+							result !== undefined &&
+							( result !== false || attribute[ 4 ] === '-' )
+						) {
+							el.setAttribute( attribute, result );
 						} else {
-							element.ref.current.setAttribute(
-								attribute,
-								result === true && attribute[ 4 ] !== '-'
-									? ''
-									: result
-							);
+							el.removeAttribute( attribute );
 						}
 					}, [] );
 				} );

test/e2e/specs/interactivity/directive-bind.spec.ts

@@ -37,12 +37,12 @@ test.describe( 'data-wp-bind', () => {
 
 	test( 'add missing checked at hydration', async ( { page } ) => {
 		const el = page.getByTestId( 'add missing checked at hydration' );
-		await expect( el ).toHaveAttribute( 'checked', '' );
+		await expect( el ).toBeChecked();
 	} );
 
 	test( 'remove existing checked at hydration', async ( { page } ) => {
 		const el = page.getByTestId( 'remove existing checked at hydration' );
-		await expect( el ).not.toHaveAttribute( 'checked', '' );
+		await expect( el ).not.toBeChecked();
 	} );
 
 	test( 'update existing checked', async ( { page } ) => {
@@ -93,4 +93,167 @@ test.describe( 'data-wp-bind', () => {
 		await expect( el ).toHaveAttribute( 'aria-expanded', 'true' );
 		await expect( el ).toHaveAttribute( 'data-some-value', 'true' );
 	} );
+
+	test.describe( 'attribute hydration', () => {
+		/**
+		 * Data structure to define a hydration test case.
+		 */
+		type MatrixEntry = {
+			/**
+			 * Test ID of the element (the `data-testid` attr).
+			 */
+			testid: string;
+			/**
+			 *  Name of the attribute being hydrated.
+			 */
+			name: string;
+			/**
+			 * Array of different values to test.
+			 */
+			values: Record<
+				/**
+				 * The type of value we are hydrating. E.g., false is `false`,
+				 * undef is `undefined`, emptyString is `''`, etc.
+				 */
+				string,
+				[
+					/**
+					 * Value that the attribute should contain after hydration.
+					 * If the attribute is missing, this value is `null`.
+					 */
+					attributeValue: any,
+					/**
+					 * Value that the HTMLElement instance property should
+					 * contain after hydration.
+					 */
+					entityPropValue: any
+				]
+			>;
+		};
+
+		const matrix: MatrixEntry[] = [
+			{
+				testid: 'image',
+				name: 'width',
+				values: {
+					false: [ null, 5 ],
+					true: [ 'true', 5 ],
+					null: [ null, 5 ],
+					undef: [ null, 5 ],
+					emptyString: [ '', 5 ],
+					anyString: [ 'any', 5 ],
+					number: [ '10', 10 ],
+				},
+			},
+			{
+				testid: 'input',
+				name: 'name',
+				values: {
+					false: [ 'false', 'false' ],
+					true: [ 'true', 'true' ],
+					null: [ '', '' ],
+					undef: [ '', '' ],
+					emptyString: [ '', '' ],
+					anyString: [ 'any', 'any' ],
+					number: [ '10', '10' ],
+				},
+			},
+			{
+				testid: 'input',
+				name: 'value',
+				values: {
+					false: [ null, 'false' ],
+					true: [ null, 'true' ],
+					null: [ null, '' ],
+					undef: [ null, '' ],
+					emptyString: [ null, '' ],
+					anyString: [ null, 'any' ],
+					number: [ null, '10' ],
+				},
+			},
+			{
+				testid: 'input',
+				name: 'disabled',
+				values: {
+					false: [ null, false ],
+					true: [ '', true ],
+					null: [ null, false ],
+					undef: [ null, false ],
+					emptyString: [ null, false ],
+					anyString: [ '', true ],
+					number: [ '', true ],
+				},
+			},
+			{
+				testid: 'input',
+				name: 'aria-disabled',
+				values: {
+					false: [ 'false', undefined ],
+					true: [ 'true', undefined ],
+					null: [ null, undefined ],
+					undef: [ null, undefined ],
+					emptyString: [ '', undefined ],
+					anyString: [ 'any', undefined ],
+					number: [ '10', undefined ],
+				},
+			},
+		];
+
+		for ( const { testid, name, values } of matrix ) {
+			test( `${ name } is correctly hydrated for different values`, async ( {
+				page,
+			} ) => {
+				for ( const type in values ) {
+					const [ attrValue, propValue ] = values[ type ];
+
+					const container = page.getByTestId( `hydrating ${ type }` );
+					const el = container.getByTestId( testid );
+					const toggle = container.getByTestId( 'toggle value' );
+
+					const hydratedAttr = await el.getAttribute( name );
+					const hydratedProp = await el.evaluate(
+						( node, propName ) => ( node as any )[ propName ],
+						name
+					);
+					expect( [ type, hydratedAttr ] ).toEqual( [
+						type,
+						attrValue,
+					] );
+					expect( [ type, hydratedProp ] ).toEqual( [
+						type,
+						propValue,
+					] );
+
+					// Only check the rendered value if the new value is not
+					// `undefined` and the attibute is neither `value` nor
+					// `disabled` because Preact doesn't update the attribute
+					// for those cases.
+					// See https://github.com/preactjs/preact/blob/099c38c6ef92055428afbc116d18a6b9e0c2ea2c/src/diff/index.js#L471-L494
+					if (
+						type === 'undef' &&
+						( name === 'value' || name === 'undefined' )
+					) {
+						return;
+					}
+
+					await toggle.click( { clickCount: 2, delay: 50 } );
+
+					// Values should be the same as before.
+					const renderedAttr = await el.getAttribute( name );
+					const renderedProp = await el.evaluate(
+						( node, propName ) => ( node as any )[ propName ],
+						name
+					);
+					expect( [ type, renderedAttr ] ).toEqual( [
+						type,
+						attrValue,
+					] );
+					expect( [ type, renderedProp ] ).toEqual( [
+						type,
+						propValue,
+					] );
+				}
+			} );
+		}
+	} );
 } );