Fix false "unsaved changes" warning from Query Loop queryId churn on load

src/index.js

@@ -64,8 +64,15 @@ addFilter(
  * Ensure each Query Loop block has a unique queryId attribute.
  *
  * WordPress core does not deduplicate queryId when blocks are duplicated.
- * This HOC generates a unique queryId from the current post ID and the
- * block's index among all query blocks on the page.
+ * This HOC only assigns a new id when a block's queryId is missing or is
+ * already claimed by an earlier query block, so genuinely duplicated ids are
+ * fixed once while already-unique ids are left untouched.
+ *
+ * It deliberately does not force a strictly index-based value on every load:
+ * doing so rewrites valid, unique ids whenever they don't line up with the
+ * current block order (e.g. after a block is deleted or reordered), which marks
+ * the post as changed the instant the editor opens and surfaces a false
+ * "unsaved changes" / "Leave site?" warning.
  */
 const withUniqueQueryId = createHigherOrderComponent( ( BlockEdit ) => {
 	return ( props ) => {
@@ -75,29 +82,61 @@ const withUniqueQueryId = createHigherOrderComponent( ( BlockEdit ) => {
 			return <BlockEdit { ...props } />;
 		}
 
-		const expectedQueryId = useSelect(
+		const uniqueQueryId = useSelect(
 			( select ) => {
+				const blockEditor = select( 'core/block-editor' );
+				const allQueryBlocks =
+					blockEditor.getBlocksByName( 'core/query' );
+				const index = allQueryBlocks.indexOf( clientId );
+
+				// Bail if the block is not (yet) part of the tree; a transient
+				// state where reassigning would be unsafe.
+				if ( index === -1 ) {
+					return null;
+				}
+
+				const currentId = attributes.queryId;
+				const idOf = ( id ) =>
+					blockEditor.getBlockAttributes( id )?.queryId;
+
+				// Keep the existing id if it is set and not already used by an
+				// earlier query block. Only missing ids or later duplicates get
+				// reassigned, so unique ids are never rewritten on load. This
+				// mirrors the server-side deduplicate_query_ids() behaviour.
+				const earlierIds = allQueryBlocks.slice( 0, index ).map( idOf );
+				if ( currentId && ! earlierIds.includes( currentId ) ) {
+					return null;
+				}
+
 				let postId = 0;
 				try {
 					postId = select( 'core/editor' )?.getCurrentPostId?.() || 0;
 				} catch {
 					postId = 0;
 				}
-				const allQueryBlocks =
-					select( 'core/block-editor' ).getBlocksByName(
-						'core/query'
-					);
-				const index = allQueryBlocks.indexOf( clientId );
-				return postId * 1000 + index + 1;
+
+				// Generate a unique id, skipping any already used by another
+				// query block on the page.
+				const otherIds = allQueryBlocks
+					.filter( ( id ) => id !== clientId )
+					.map( idOf );
+				let candidate = postId * 1000 + index + 1;
+				while ( otherIds.includes( candidate ) ) {
+					candidate++;
+				}
+				return candidate;
 			},
-			[ clientId ]
+			[ clientId, attributes.queryId ]
 		);
 
 		useEffect( () => {
-			if ( attributes.queryId !== expectedQueryId ) {
-				setAttributes( { queryId: expectedQueryId } );
+			if (
+				uniqueQueryId !== null &&
+				attributes.queryId !== uniqueQueryId
+			) {
+				setAttributes( { queryId: uniqueQueryId } );
 			}
-		}, [ expectedQueryId, attributes.queryId, setAttributes ] );
+		}, [ uniqueQueryId, attributes.queryId, setAttributes ] );
 
 		return <BlockEdit { ...props } />;
 	};

tests/e2e/unique-query-id.spec.js

@@ -118,8 +118,6 @@ test.describe( 'Unique Query IDs', () => {
 			} );
 		} );
 
-		console.log( 'Query IDs in editor:', queryIds );
-
 		// Verify we have 3 query loops
 		expect( queryIds ).toHaveLength( 3 );
 
@@ -131,10 +129,135 @@ test.describe( 'Unique Query IDs', () => {
 
 		// Verify all queryIds are unique
 		const uniqueIds = [ ...new Set( queryIds ) ];
-		console.log( 'Unique query IDs:', uniqueIds );
 		expect( uniqueIds ).toHaveLength( 3 );
 	} );
 
+	test( 'should not mark post as dirty when editor reloads with already-unique queryIds', async ( {
+		page,
+		admin,
+	} ) => {
+		// Create a new page with a Query Loop block
+		await admin.createNewPost( { postType: 'page' } );
+		await page.waitForTimeout( 1500 );
+
+		const closeButton = page.getByRole( 'button', { name: 'Close' } );
+		if (
+			await closeButton
+				.isVisible( { timeout: 1000 } )
+				.catch( () => false )
+		) {
+			await closeButton.click();
+		}
+
+		const canvas = page
+			.locator( 'iframe[name="editor-canvas"]' )
+			.contentFrame();
+
+		await canvas.getByRole( 'textbox', { name: 'Add title' } ).click();
+		await canvas
+			.getByRole( 'textbox', { name: 'Add title' } )
+			.fill( 'Test no dirty on load' );
+
+		await canvas
+			.getByRole( 'button', { name: 'Add default block' } )
+			.click();
+		await canvas
+			.getByRole( 'document', {
+				name: 'Empty block; start writing or',
+			} )
+			.fill( '/query' );
+		await page
+			.getByRole( 'option', { name: 'Query Loop' } )
+			.first()
+			.click();
+		await page.waitForTimeout( 1000 );
+
+		const startBlankButton = canvas.getByRole( 'button', {
+			name: 'Start blank',
+		} );
+		if (
+			await startBlankButton
+				.isVisible( { timeout: 2000 } )
+				.catch( () => false )
+		) {
+			await startBlankButton.click();
+			await page.waitForTimeout( 500 );
+		}
+
+		const patternButton = canvas.getByRole( 'button', {
+			name: 'Title & Date',
+		} );
+		if (
+			await patternButton
+				.isVisible( { timeout: 2000 } )
+				.catch( () => false )
+		) {
+			await patternButton.click();
+			await page.waitForTimeout( 500 );
+		}
+
+		// Give the HOC time to assign a queryId
+		await page.waitForTimeout( 1000 );
+
+		const initialQueryId = await page.evaluate( () => {
+			const blocks = window.wp.data
+				.select( 'core/block-editor' )
+				.getBlocksByName( 'core/query' );
+			const block = window.wp.data
+				.select( 'core/block-editor' )
+				.getBlock( blocks[ 0 ] );
+			return block.attributes.queryId;
+		} );
+
+		expect( typeof initialQueryId ).toBe( 'number' );
+
+		// Publish the page
+		await page
+			.getByRole( 'button', { name: 'Publish', exact: true } )
+			.click();
+		await page
+			.getByLabel( 'Editor publish' )
+			.getByRole( 'button', { name: 'Publish', exact: true } )
+			.click();
+		await page.waitForTimeout( 1500 );
+
+		const postId = await page.evaluate( () => {
+			return window.wp.data.select( 'core/editor' ).getCurrentPostId();
+		} );
+
+		// Reopen the editor — this is the scenario where the false dirty
+		// warning appeared before the fix: the HOC was rewriting the
+		// queryId on every load, marking the post as changed immediately.
+		await admin.visitAdminPage( `post.php?post=${ postId }&action=edit` );
+		await page.waitForSelector( 'iframe[name="editor-canvas"]', {
+			timeout: 15000,
+		} );
+		// Wait for Gutenberg to fully initialise and for all useEffect
+		// callbacks (including our withUniqueQueryId HOC) to run.
+		await page.waitForTimeout( 2000 );
+
+		const reloadedQueryId = await page.evaluate( () => {
+			const blocks = window.wp.data
+				.select( 'core/block-editor' )
+				.getBlocksByName( 'core/query' );
+			const block = window.wp.data
+				.select( 'core/block-editor' )
+				.getBlock( blocks[ 0 ] );
+			return block.attributes.queryId;
+		} );
+
+		// The queryId must not have changed — stable ids prevent rewriting.
+		expect( reloadedQueryId ).toBe( initialQueryId );
+
+		// The editor must not be dirty; any attribute rewrite on load would
+		// have triggered the "unsaved changes / Leave site?" warning.
+		const isDirty = await page.evaluate( () => {
+			return window.wp.data.select( 'core/editor' ).isEditedPostDirty();
+		} );
+
+		expect( isDirty ).toBe( false );
+	} );
+
 	test( 'should have unique queryId in saved content and exclude posts correctly on front end', async ( {
 		page,
 		admin,
@@ -238,7 +361,6 @@ test.describe( 'Unique Query IDs', () => {
 			} );
 		} );
 
-		console.log( 'Query IDs before publish:', queryIds );
 		expect( queryIds ).toHaveLength( 2 );
 		const uniqueIds = [ ...new Set( queryIds ) ];
 		expect( uniqueIds ).toHaveLength( 2 );
@@ -251,17 +373,13 @@ test.describe( 'Unique Query IDs', () => {
 			.locator( '.wp-block-post-template .wp-block-post-title' )
 			.allTextContents();
 
-		console.log( 'All post titles on page:', allPostTitles );
-		console.log( 'Total posts:', allPostTitles.length );
-
 		// Both query loops should have posts (each shows 10 by default)
 		expect( allPostTitles.length ).toBe( 20 );
 
 		// With exclude displayed enabled on both and unique queryIds,
 		// the second loop should exclude posts from the first.
 		// All 20 titles should be unique.
 		const uniqueTitles = [ ...new Set( allPostTitles ) ];
-		console.log( 'Unique posts:', uniqueTitles.length );
 		expect( uniqueTitles ).toHaveLength( 20 );
 	} );
 } );