Skip to content

Fix editor hang from NaN query ID seed in the site editor - #26

Merged
michelhm merged 1 commit into
mainfrom
fix/query-id-seed-in-site-editor
Jul 21, 2026
Merged

michelhm merged 1 commit into
mainfrom
fix/query-id-seed-in-site-editor

Conversation

@michelhm

@michelhm michelhm commented Jul 21, 2026 •

Copy link
Copy Markdown
Member

Problem

withUniqueQueryId seeds ids with postId * 1000. In the site editor getCurrentPostId() returns the template's string id (twentytwentyfive//archive), so the seed is NaN.

That hangs the editor, because [ NaN ].includes( NaN ) is true (SameValueZero) and NaN++ stays NaN — so the dedupe loop added in 64d4c9f never terminates. It also rewrites queryId on every render, since NaN !== NaN.

Triggered by any template with two or more query blocks whose ids are missing or duplicated.

Fix

Coerce the seed to a finite number, hashing non-numeric ids into a stable integer. Finiteness is what makes the loop terminate; stability keeps ids from being rewritten on load. Post editor behaviour is unchanged.

withUniqueQueryId seeds generated ids with `postId * 1000`, where postId comes
from `getCurrentPostId()`. In the post editor that is a numeric post ID, but in
the site editor it is the template's string id (`theme//archive-opinion`), so
the seed is NaN.

NaN then poisons both comparisons the HOC relies on:

- `NaN !== NaN` is always true, so the useEffect rewrote queryId on every
  render, marking the template dirty.
- `[ NaN ].includes( NaN )` is true (includes uses SameValueZero, unlike
  indexOf) and `NaN++` stays NaN, so once any block on the page held a NaN
  queryId, the dedupe loop added in 64d4c9f never terminated and hung the
  editor.

Any template with two or more query blocks whose ids are missing or duplicated
reaches that loop, which is how it surfaced: block themes commonly carry
patterns with hardcoded low queryIds alongside templates that omit them.

Coerce the seed to a finite number, hashing non-numeric ids into a stable
integer. Stability matters as much as finiteness here: a seed that changed
between loads would rewrite ids on open and reintroduce the false "unsaved
changes" warning 64d4c9f fixed. Post editor behaviour is unchanged, and a
finite seed is what guarantees the dedupe loop terminates.

The existing queryId tests all run in the post editor, so none covered the
string-id path. Add a site editor test asserting ids are finite, unique and
stable; it asserts `typeof templateId === 'string'` up front so it fails loudly
rather than silently stops covering the bug if core ever returns a numeric id.
Number.isFinite is the operative check, since typeof NaN is 'number'.
@github-actions

Copy link
Copy Markdown

Playwright test results

passed  20 passed

Details

stats  20 tests across 6 suites
duration  1 minute, 11 seconds
commit  37a7cee

@michelhm
michelhm requested a review from mattheu July 21, 2026 15:49
@michelhm michelhm self-assigned this Jul 21, 2026

@mattheu mattheu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good fix.

Make sure to test carefully on a few templates - refer to the testing checklist in confluence and make sure all of these work OK.

@michelhm
michelhm requested a review from roborourke July 21, 2026 15:59
@michelhm
michelhm merged commit 766763b into main Jul 21, 2026
1 check passed
@michelhm
michelhm deleted the fix/query-id-seed-in-site-editor branch July 21, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants