ReviewOS

stacks/stx

:if removes a child component's markup but still runs its onMount, so "mounted means open" cannot work

#2036
Closed glennmichael123 opened this 10 days ago ·
10 days ago

:if on a wrapper removes a child component's markup but still runs its onMount, so a component cannot use being mounted as its "open" state.

Measured on stx 0.2.368.

Repro

Three files. The child logs when it mounts:

<!-- components/gate-child.stx -->
<script server>
export const which = $props.which || 'unknown'
</script>

<script client>
const which = {{ which }} ?? 'unknown'
onMount(() => { console.log('[gate] mounted:', which) })
onDestroy(() => { console.log('[gate] destroyed:', which) })
</script>

<div class="gate-child">{{ which }}</div>
<!-- components/gate-local.stx — a partial with its own signal -->
<script client>
  const gateLocalOpen = state(false)
</script>

<div :if="gateLocalOpen">
  <GateChild which="partial-local-signal" />
</div>
<!-- views/gate.stx -->
<script client>
  const gateHostOpen = state(false)
</script>

<div>
  <!-- A: directly in the view, gated by the view's own signal -->
  <div :if="gateHostOpen">
    <GateChild which="view-direct" />
  </div>

  <!-- B: in a partial, gated by a signal the partial declares -->
  @include('gate-local')
</div>

Both signals are false and never set. Load the page:

[gate] mounted: view-direct
[gate] mounted: partial-local-signal
document.querySelectorAll('.gate-child').length   // 0

So the :if does what it says to the markup — nothing is rendered — while the component's <script client> lifecycle runs anyway. onDestroy does not fire either, so the component is mounted and stays mounted, invisibly.

Why it matters

A component whose job starts at mount has no way to be conditionally active. Ours is a modal shell: it moves focus into its panel, traps Tab, locks body scroll and registers an Escape handler in onMount, and releases all of that in onDestroy. Gating it from outside is the documented way to avoid :open as a prop, which has its own problem in a partial (#1994).

With the lifecycle running regardless, a page with three of these behind :if locks the document's scroll before anyone has opened anything, and Escape reaches whichever instance registered last rather than the one on screen. The symptom is a page you cannot scroll with no dialog on it.

The workaround is to go back to a bound :open prop and have the component do nothing while it is false — which means the component is always mounted, which is what gating from outside was for.

What would fix it

:if should govern the child component's lifecycle, not only its output: no mount while the condition is false, onDestroy when it becomes false.

If that is a larger change than it sounds, a documented way for a component to ask "am I actually rendered?" — or for :if to defer the mount — would be enough to build on.

Note on a wrong turn, in case it narrows anything

I first thought this was about partials included from a layout specifically, because in our app the symptom appeared on pages whose layout includes the partial and not on a page whose view includes one. The minimal repro above does not support that: case A, a component directly in a view, mounts just the same. Whatever makes the app's two cases differ is probably timing rather than placement, and the mount-regardless behaviour is the part that reproduces cleanly.

10 days ago

Fixed in 0.2.380 — with one half of your ask delivered and the other deliberately not, described below so you can decide whether it needs its own issue.

Cause

Two of them, for the two placements you reported.

A component whose client script is nothing but lifecycle hooks got no scope boundary at all. hasSignalScripts listed state, derived and the composables but not onMount/onDestroy, so the component's hooks went to the runtime's global mount queue — a list with no link to any element — which the DOMContentLoaded handler drains unconditionally. There was nothing to check a condition against, which is why the behaviour was so absolute. The same two names were already in SIGNAL_API_RE, which decides the identical question for a page or layout script; the two lists disagreeing is why this was reachable from a component and not from a page.

That alone does not fix the partial case, because the partial's own scope is on the page, so the scope walk reaches the child and has to notice its root is detached. Measured both ways: with the boundary and no detachment check, your case B still mounts.

Your note about the wrong turn was the useful part of the report — both placements do reproduce, and it turned out they needed different fixes, so the distinction you thought was wrong was half right for a reason you could not have seen from outside.

What is fixed

No mount while the condition is false, in both placements, and the mount is deferred rather than suppressed: bindIf showing the subtree flushes it, through the same path stx-hydrate islands already use. Verified in a harness that drives the real runtime — shows and renders on open, does not mount twice on re-show, and an ungated component on the same page still mounts as a control.

What is not fixed

Hiding does not fire onDestroy, and a second show does not mount again. bindIf deliberately does not dispose scopes on hide, because :if is a toggle and #1737 needs the registration to survive the re-show.

So this is the fallback you named as sufficient — "or for :if to defer the mount" — and not full symmetry. For your modal shell that means it no longer locks body scroll or claims Escape before anything is opened, which was the reported symptom; but a modal that has been opened once will not release those on close through onDestroy. Until symmetry lands, a component that must release something on hide still needs a bound prop it can watch.

Happy to pursue the symmetric version if you want it tracked — it touches the part of the runtime that #1737, #1954 and #1773 all warn about, so it is worth doing deliberately rather than as part of this.

Regression test: test/router/conditional-component-lifecycle.test.ts, which also states the asymmetry above rather than leaving it to be discovered.

Sign in to comment on this issue.