Fix editor hang from NaN query ID seed in the site editor

src/index.js

@@ -60,6 +60,40 @@ addFilter(
 	addQueryLoopAttributes
 );
 
+/**
+ * Derive a numeric seed for generated query IDs from the current post ID.
+ *
+ * In the post editor `getCurrentPostId()` returns a numeric post ID, but in the
+ * site editor it returns the template's string id (`theme//archive-opinion`),
+ * for which `postId * 1000` is NaN. NaN poisons every comparison downstream:
+ * `NaN !== NaN` is always true, so the id would be rewritten on every render,
+ * and `[ NaN ].includes( NaN )` is true, so the dedupe loop below would spin
+ * forever and hang the editor.
+ *
+ * Non-numeric ids are hashed to a stable integer instead. Stability matters: a
+ * seed that changed between loads would rewrite ids on open and reintroduce the
+ * false "unsaved changes" warning this HOC exists to avoid.
+ *
+ * @param {number|string} postId Current post or template ID.
+ * @return {number} A finite, stable numeric seed.
+ */
+const getQueryIdSeed = ( postId ) => {
+	const numericId = Number( postId );
+
+	if ( Number.isFinite( numericId ) ) {
+		return numericId;
+	}
+
+	let hash = 0;
+	const key = String( postId );
+	for ( let i = 0; i < key.length; i++ ) {
+		// Bounded each round so the seed stays a small, exact integer.
+		hash = ( hash * 31 + key.charCodeAt( i ) ) % 100000;
+	}
+
+	return hash;
+};
+
 /**
  * Ensure each Query Loop block has a unique queryId attribute.
  *
@@ -116,11 +150,12 @@ const withUniqueQueryId = createHigherOrderComponent( ( BlockEdit ) => {
 				}
 
 				// Generate a unique id, skipping any already used by another
-				// query block on the page.
+				// query block on the page. The seed is always finite, which is
+				// what guarantees this loop terminates.
 				const otherIds = allQueryBlocks
 					.filter( ( id ) => id !== clientId )
 					.map( idOf );
-				let candidate = postId * 1000 + index + 1;
+				let candidate = getQueryIdSeed( postId ) * 1000 + index + 1;
 				while ( otherIds.includes( candidate ) ) {
 					candidate++;
 				}

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

@@ -382,4 +382,85 @@ test.describe( 'Unique Query IDs', () => {
 		const uniqueTitles = [ ...new Set( allPostTitles ) ];
 		expect( uniqueTitles ).toHaveLength( 20 );
 	} );
+
+	test( 'should assign finite, unique queryIds in the site editor', async ( {
+		page,
+		editor,
+		blockEditor,
+	} ) => {
+		// The site editor's "post ID" is the template's string id
+		// (`theme//slug`), not a number. Generating ids from it without
+		// coercing to a number yields NaN, and because `NaN !== NaN` and
+		// `[ NaN ].includes( NaN )` is true, that previously rewrote the
+		// attribute on every render and hung the editor in the dedupe loop.
+		await blockEditor.visitSiteEditor( 'index' );
+
+		const templateId = await page.evaluate( () =>
+			window.wp.data.select( 'core/editor' ).getCurrentPostId()
+		);
+
+		// Guard the premise of this test: if core ever starts handing the site
+		// editor a numeric id, this test would silently stop covering the bug.
+		expect( typeof templateId ).toBe( 'string' );
+
+		// Two query blocks sharing a queryId. The first keeps its id; the
+		// second is a duplicate, so it takes the id-generation path.
+		await editor.setContent(
+			`<!-- wp:query {"queryId":10,"query":{"perPage":3,"postType":"post","inherit":false}} -->
+<div class="wp-block-query"><!-- wp:post-template -->
+<!-- wp:post-title /-->
+<!-- /wp:post-template --></div>
+<!-- /wp:query -->
+
+<!-- wp:query {"queryId":10,"query":{"perPage":3,"postType":"post","inherit":false}} -->
+<div class="wp-block-query"><!-- wp:post-template -->
+<!-- wp:post-title /-->
+<!-- /wp:post-template --></div>
+<!-- /wp:query -->`
+		);
+
+		// Let the HOC settle. A regression hangs the editor here, so the
+		// evaluate below times out rather than returning a wrong value.
+		await page.waitForTimeout( 2000 );
+
+		const queryIds = await page.evaluate( () => {
+			const { select } = window.wp.data;
+			return select( 'core/block-editor' )
+				.getBlocksByName( 'core/query' )
+				.map(
+					( clientId ) =>
+						select( 'core/block-editor' ).getBlockAttributes(
+							clientId
+						).queryId
+				);
+		} );
+
+		expect( queryIds ).toHaveLength( 2 );
+
+		// The regression assertion: NaN is a number but not a finite one, so
+		// `typeof` alone would not have caught it.
+		for ( const queryId of queryIds ) {
+			expect( Number.isFinite( queryId ) ).toBe( true );
+		}
+
+		expect( [ ...new Set( queryIds ) ] ).toHaveLength( 2 );
+
+		// Ids must also be stable once assigned — an id that keeps changing
+		// marks the template dirty on every render.
+		await page.waitForTimeout( 1000 );
+
+		const settledQueryIds = await page.evaluate( () => {
+			const { select } = window.wp.data;
+			return select( 'core/block-editor' )
+				.getBlocksByName( 'core/query' )
+				.map(
+					( clientId ) =>
+						select( 'core/block-editor' ).getBlockAttributes(
+							clientId
+						).queryId
+				);
+		} );
+
+		expect( settledQueryIds ).toEqual( queryIds );
+	} );
 } );