Interactivity API: Fix popover bind hydration

Author: cbravobernal

package-lock.json

@@ -98,6 +98,35 @@
 	</div>
 	<?php endforeach; ?>
 
+	<?php
+	$popover_cases = array(
+		'true'   => '{ "value": true }',
+		'false'  => '{ "value": false }',
+		'null'   => '{ "value": null }',
+		'undef'  => '{ "__any": "any" }',
+		'auto'   => '{ "value": "auto" }',
+		'manual' => '{ "value": "manual" }',
+		'hint'   => '{ "value": "hint" }',
+	);
+	?>
+
+	<?php foreach ( $popover_cases as $type => $context ) : ?>
+	<div
+		data-testid='hydrating popover <?php echo $type; ?>'
+		data-wp-context='<?php echo $context; ?>'
+	>
+		<div
+			data-testid="popover"
+			data-wp-bind--popover="context.value"
+		></div>
+		<button
+			data-testid="toggle value"
+			data-wp-on--click="actions.toggleValue"
+			data-wp-bind--data-toggle-count="context.count"
+		>Toggle</button>
+	</div>
+	<?php endforeach; ?>
+
 	<div data-wp-context='{"test": true}'>
 		<div
 			data-testid="without-unique-id"

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

@@ -35,7 +35,7 @@
 	"types": "build-types",
 	"dependencies": {
 		"@preact/signals": "^1.3.0",
-		"preact": "^10.24.2"
+		"preact": "10.29.1"
 	},
 	"devDependencies": {
 		"@types/jest": "^29.5.14"

packages/interactivity/package.json

@@ -745,7 +745,7 @@ export default () => {
 				/*
 				 * 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
+				 * logic: https://github.com/preactjs/preact/blob/10.29.1/src/diff/props.js#L115-L129
 				 */
 				if ( attribute === 'style' ) {
 					if ( typeof result === 'string' ) {
@@ -774,6 +774,7 @@ export default () => {
 					attribute !== 'rowSpan' &&
 					attribute !== 'colSpan' &&
 					attribute !== 'role' &&
+					attribute !== 'popover' &&
 					attribute in el
 				) {
 					try {
@@ -788,14 +789,21 @@ export default () => {
 				 * 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
+				 * We follow Preact's logic: https://github.com/preactjs/preact/blob/10.29.1/src/diff/props.js#L138-L150
 				 */
+				if ( typeof result === 'function' ) {
+					return;
+				}
+
 				if (
 					result !== null &&
 					result !== undefined &&
 					( result !== false || attribute[ 4 ] === '-' )
 				) {
-					el.setAttribute( attribute, result );
+					el.setAttribute(
+						attribute,
+						attribute === 'popover' && result === true ? '' : result
+					);
 				} else {
 					el.removeAttribute( attribute );
 				}

packages/interactivity/src/directives.tsx

@@ -127,6 +127,18 @@ test.describe( 'data-wp-bind', () => {
 					 * contain after hydration.
 					 */
 					entityPropValue: any,
+					/**
+					 * Value that the attribute should contain after Preact
+					 * renders the same value. If omitted, the hydration value
+					 * is used.
+					 */
+					renderedAttributeValue?: any,
+					/**
+					 * Value that the HTMLElement instance property should
+					 * contain after Preact renders the same value. If omitted,
+					 * the hydration value is used.
+					 */
+					renderedEntityPropValue?: any,
 				]
 			>;
 		};
@@ -164,8 +176,8 @@ test.describe( 'data-wp-bind', () => {
 				values: {
 					false: [ null, 'false' ],
 					true: [ null, 'true' ],
-					null: [ null, '' ],
-					undef: [ null, '' ],
+					null: [ null, '', null, 'tacocat' ],
+					undef: [ null, '', null, 'tacocat' ],
 					emptyString: [ null, '' ],
 					anyString: [ null, 'any' ],
 					number: [ null, '10' ],
@@ -204,7 +216,12 @@ test.describe( 'data-wp-bind', () => {
 				page,
 			} ) => {
 				for ( const type in values ) {
-					const [ attrValue, propValue ] = values[ type ];
+					const [
+						attrValue,
+						propValue,
+						renderedAttrValue = attrValue,
+						renderedPropValue = propValue,
+					] = values[ type ];
 
 					const container = page.getByTestId( `hydrating ${ type }` );
 					const el = container.getByTestId( testid );
@@ -230,18 +247,6 @@ test.describe( 'data-wp-bind', () => {
 						propValue,
 					] );
 
-					// Only check the rendered value if the new value is not
-					// `undefined` and the attribute 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 } );
 
 					// Ensure values have been updated after toggling.
@@ -250,23 +255,65 @@ test.describe( 'data-wp-bind', () => {
 						'2'
 					);
 
-					// 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,
+						renderedAttrValue,
 					] );
 					expect( [ type, renderedProp ] ).toEqual( [
 						type,
-						propValue,
+						renderedPropValue,
 					] );
 				}
 			} );
 		}
+
+		test( 'popover is correctly hydrated for different values', async ( {
+			page,
+		} ) => {
+			const values: Record< string, string | null > = {
+				true: '',
+				false: null,
+				null: null,
+				undef: null,
+				auto: 'auto',
+				manual: 'manual',
+				hint: 'hint',
+			};
+
+			// Ensure hydration has happened.
+			const checkbox = page.getByTestId(
+				'add missing checked at hydration'
+			);
+			await expect( checkbox ).toBeChecked();
+
+			for ( const type in values ) {
+				const attrValue = values[ type ];
+				const container = page.getByTestId(
+					`hydrating popover ${ type }`
+				);
+				const el = container.getByTestId( 'popover' );
+				const toggle = container.getByTestId( 'toggle value' );
+
+				const hydratedAttr = await el.getAttribute( 'popover' );
+				expect( [ type, hydratedAttr ] ).toEqual( [ type, attrValue ] );
+
+				await toggle.click( { clickCount: 2 } );
+
+				// Ensure values have been updated after toggling.
+				await expect( toggle ).toHaveAttribute(
+					'data-toggle-count',
+					'2'
+				);
+
+				const renderedAttr = await el.getAttribute( 'popover' );
+				expect( [ type, renderedAttr ] ).toEqual( [ type, attrValue ] );
+			}
+		} );
 	} );
 
 	test( 'should ignore unique ids', async ( { page } ) => {