ReviewOS

stacks/stx

<template :for> directly under <template :if> leaves its rows on screen after the branch hides

#1955
Closed glennmichael123 opened this 10 days ago · {{ comments.length }} {{ comments.length === 1 ? 'comment' : 'comments' }}
10 days ago

A <template :for> placed directly inside a <template :if> keeps its rendered rows on screen after the :if becomes false. The branch is hidden, but its list is still visible.

Found while verifying the #1954 fix. It is not caused by that fix: unmodified main at db32fc6618 behaves the same way, and so does 092ed2d12d.

Reproduction

<div id="host">
  <template :if="shown()">
    <template :for="i in items" :key="i">
      <b class="row">{{ i }}</b>
    </template>
  </template>
</div>

With items = [1, 2, 3], set shown to false.

  • Expected: no .row elements in #host.
  • Observed: all 3 .row elements remain.

Measured under happy-dom with the full client runtime, the same harness as packages/stx/test/signals/dispose-keeps-live-bindings.test.ts. Not yet checked in a real browser.

What does work

The same list wrapped in an element hides correctly:

<template :if="shown()">
  <ul><template :for="i in items" :key="i"><li class="row">{{ i }}</li></template></ul>
</template>

dispose-keeps-live-bindings.test.ts pins that variant across five hide/show cycles.

Likely cause (from reading the code, not confirmed)

Hiding a <template :if> removes currentNodes, the clones of el.content, in bindIf's template branch in packages/stx/src/signals.ts. A :for that is a direct child of the template renders its rows as siblings of its own clone, not inside it. The rows are not among currentNodes, so hide never removes them. Wrapping the list works because the wrapper is a clone and the rows are its descendants.

A fix probably needs the template :if to own whatever its direct-child structural directives insert: for example, removing the nodes between its placeholder and the end of its inserted range, rather than only the clones it created.

Test to add

This case was written for #1954 and taken out because it fails on main. It slots into dispose-keeps-live-bindings.test.ts, which provides mount, hits and settle:

it('removes rows of a <template :for> placed DIRECTLY under a hidden <template :if>', async () => {
  const shown = window.stx.state(true)
  const items = window.stx.state([1, 2, 3])
  const page = await mount(`
    <div id="host"><template :if="shown()"><template :for="i in items" :key="i"><b class="row" x-class="probe(i)">{{ i }}</b></template></template></div>`, { shown, items })

  expect(hits('#host .row')).toBe(3)
  for (let cycle = 0; cycle < 5; cycle++) {
    shown.set(false)
    await settle()
    expect(document.querySelectorAll('#host .row').length).toBe(0)
    expect(await page.oneUpdate()).toBe(0)
    shown.set(true)
    await settle()
    expect(hits('#host .row')).toBe(3)
    expect(await page.oneUpdate()).toBe(3)
  }
})

On main it fails at the first hide: expected 0 rows, received 3.

🤖 Generated with Claude Code

10 days ago

Fixed in 7fc1393d7f.

bindIf now marks where a branch's rendered content ends, at both points that insert it — the initial bind and every re-show — and hiding removes the range between the placeholder and that marker, disposing effects on the way (#1954). bindFor already carries per-item marker comments for the same reason, so the shape is not new to the runtime.

The test written for this during #1954, and pulled back out because it failed on unmodified main, now lives in dispose-keeps-live-bindings.test.ts. It fails against the parent commit.

Two wrong turns, recorded because the second is the more interesting one:

The marker went into the wrong branch. The anchor line it matched was unique to the single-element branch, while the removal walked the template branch — so the marker was always null there and the walk ran to the end of the parent, deleting a sibling <template :if> and its content. Two projected-slot tests in template-for-dom.test.ts caught it. Notably the #1955 case itself passed the whole time: deleting everything to the end of the parent looks correct when the branch happens to be last.

Guarding the walk on "a marker exists" then broke #1955 again. The first hide of a branch that was never toggled removes content inserted at bind time, which had no marker. Hence marking at both insertion points rather than only on re-show.

Suite 12,903 pass / 0 fail; CI green.

🤖 Generated with Claude Code

Sign in to comment on this issue.