fix(core): keep later siblings attached when an earlier element is removed

[ybai08]

What is it?

• Bug

Description

Fixes #9188.

When a conditional element is removed, every later sibling of a different tag is detached and re-inserted. An <iframe> after it reloads, and a focused element after it loses focus:

<section>
  {show.value && <p>Message</p>}
  <iframe srcdoc={frame} />
  <button onClick$={() => (show.value = !show.value)}>Toggle message</button>
</section>

Cause: in moveOrCreateKeyedNode, when an unkeyed node is found further down the siblings (<iframe> while the cursor is still on the stale <p>), the found node is moved in front of the cursor. The <p> is deleted a moment later by expectNoMore, so every kept sibling is moved once just to step over a node that is about to go away.

Fix: when the found node is a later sibling at the same level, move the siblings that were skipped to reach it to just after it, instead of moving the found node. The resulting vnode order is exactly what it was before ([found, skipped..., rest]), so the rest of the diff runs unchanged; only the DOM operations differ. In the issue's case the doomed <p> is the node that moves, and the <iframe> and <button> are never touched. Skipped text vnodes are inflated first, as vnode_remove does. If the found node is not a later sibling at this level, the old move is used.

A genuine reorder of unkeyed elements of different tags still works. It may now move the skipped run instead of the single found node. Consecutive moves before the same node are batched into one fragment insert by _flushJournal.

Tests, in vnode-diff.unit.tsx, check the journal directly, like reconcile-keyed-loop.unit.tsx does:

• {false} replacing <p> before <iframe/><button/>: no insert/move or delete operation targets the iframe or the button (fails on main);
• the <p> and a <span> dropped from before <iframe/><button/>: same check (fails on main);
• unkeyed <p><iframe><button> reordered to <button><p><iframe>: same DOM nodes, right order (passes before and after).

Commands run (Node 22, pnpm build.core.dev first):

• pnpm vitest run packages/qwik/src/core/client/vnode-diff.unit.tsx -t "removing an earlier element": 2 failed, 1 passed with the fix reverted; 3 passed with it.
• pnpm vitest run packages/: 3855 passed, 7 failed. The 7 failures are all in qwik-vite/src/plugins/plugin.unit.ts and also fail on main in this environment (Cannot find module .../bindings/qwik.wasm.mjs, no Rust/WASM build). On main the same run gives 3852 passed, 7 failed. Packages whose dependencies were not installed (devtools, docs, mcp, browser-extension) fail to import in both runs.
• pnpm test.e2e.chromium (dev build): 580 passed, 1 failed, 2 skipped, 1 did not run. The failure is ssg-snapshot.e2e.ts › "core chunk brotli size stays within budget". It also fails on main with the dev build (36783 bytes on main, 36821 on this branch, budget 34800). The test itself says it expects the production build, which was not built here.
• prettier --check and eslint on the changed files: clean. tsc --noEmit reports only missing react-dom / @supabase/* modules from packages that were not installed. Nothing in packages/qwik.

Not run: pnpm build.core / api.update (no public API change), the Rust optimizer, the e2e suites with QWIK_OPTIMIZER=rust, the production build for the brotli budget check.

Checklist

• My code follows the developer guidelines of this project
• I performed a self-review of my own code
• I added a changeset with pnpm change
• I added new tests to cover the fix / functionality

[changeset-bot]

🦋 Changeset detected

Latest commit: 51b7c19

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
@qwik.dev/core	Patch
@qwik.dev/devtools	Patch
eslint-plugin-qwik	Patch
@qwik.dev/mcp	Patch
@qwik.dev/react	Patch
@qwik.dev/router	Patch
@qwik.dev/utils	Patch
create-qwik	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Varixo]

Hey, thanks for the PR, but I think the approach fixes only your case and still can rerender unkeyed nodes in many different cases.
We could use existing side buffer. We could leave the sibling links as they are, use the buffer to track the new order. Once matching is done, remove the nodes we didn’t reuse and compare both ends to see which nodes actually need to move. So this should work for keyed and nonkeyed nodes.

Also please cover both cases: removing

from [p, iframe, button] should leave iframe/button untouched, and rotating [p, iframe, button] to [button, p, iframe] and back should move only the button.

We can change vSideBuffer to
$vSideBuffer$: Map<VNode, number> | null;
where negative entries are unmatched

Here is simple pseudo code:

while matching, if the side buffer is active:
    buffer.delete(matchedNode)
    buffer.set(matchedNode, 0)
    preserve the old sibling links

after matching:
    remove unmatched nodes

    // old order: VNode links, new order: buffer order
    while buffer is not empty:
        if oldFirst == newFirst:
            accept start
        else if oldLast == newLast:
            accept end
        else if oldLast == newFirst:
            move oldLast before oldFirst
            accept start
        else if oldFirst == newLast:
            move oldFirst after oldLast
            accept end
        else:
            move newFirst before oldFirst
            accept start

// accept = remove from buffer and advance the corresponding pointers

Let me know if this is clear

[ybai08]

Thanks for the detailed pointer, that was clear. Reworked along those lines and rebased on main (force-pushed as one commit):

• $vSideBuffer$ is now Map<VNode, number> | null (-1 = unmatched). It is filled when the siblings are first scanned; advance() re-inserts each matched node with 0, so the Map order is the new order, and found nodes stay in place (lookups and the cursor skip already-matched siblings).
• After matching, cleanupSideBuffer removes the unmatched nodes and then reorders the rest with the two-ended comparison from your pseudocode.
• This replaces the old string-keyed side-buffer path (collectSideBufferSiblings, deleteFromSideBuffer, the side-insert branch), so keyed and unkeyed nodes go through the same code. vnode-diff.ts is about 80 lines shorter.

Tests in vnode-diff.unit.tsx ("keeping siblings in place"): removing <p> from [p, iframe, button] leaves iframe/button untouched (no insert/move/delete in the journal, same DOM nodes), and rotating [p, iframe, button] → [button, p, iframe] and back moves only the button, both unkeyed and keyed. On main all 4 fail; with the previous version of this PR the 2 rotation cases fail; now all pass, and the whole file is 95/95. vitest run packages/qwik/src/core: only async-local-storage.unit.ts fails, the same 3 tests as on main in this local setup.

One open point: new nodes are still inserted at the cursor while matching, so a new node created out of place can be inserted and then moved once at the end. Happy to change that if you'd prefer another approach.

[pkg-pr-new]

Open in StackBlitz

@qwik.dev/core

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@9229

@qwik.dev/router

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@9229

@qwik.dev/utils

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/utils@9229

eslint-plugin-qwik

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@9229

create-qwik

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@9229

@qwik.dev/optimizer

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/optimizer@9229

@qwik.dev/devtools

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/devtools@9229

@qwik.dev/mcp

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/mcp@9229

commit: 51b7c19