ReviewOS

stacks/stx

fix(stx-native): `:if` on an interactive node renders it with a dead handler, silently

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

The gap

:if on an interactive node in a native screen produces a node that renders, is tappable, and whose handler is dead. No error, no warning, no diagnostic from the compiler.

Measured on stx 0.2.448, wildloop's resources/native/Notifications.stx:

<Button :if="unread() > 0" testID="mark-all" @click="markAllRead()">Mark all read</Button>

What the host is sent, in order:

  1. The first RENDER contains the node — testID="mark-all" is in the document before unread() is ever greater than zero.
  2. When the condition turns true, no createNode arrives for it. Checked by filtering every MUTATE operation for node.props.testID === 'mark-all': zero.
  3. The handler the host holds for that id is the one registered against the node from step 1. Dispatching an EVENT for it produces nothing at all — no mutation, no capability request, no error.

So the button is on screen, the tap is accepted, and the function never runs.

Why it took a while to find

The failure has no signature. What it looked like from the outside was a hung capability: the screen's write never issued a request, so the obvious suspect was secureStorage.get, which the write calls first. That is a red herring — the bridge gives every request a new message id and holds no cache, verified by driving two reads of the same key through it. Nothing was hung. The tap simply never ran.

A rule that silently drops a handler costs more than the same rule throwing, because the cheap explanations are all wrong.

What works

  • :if on a Text node behaves correctly, including when the condition flips. Every one of wildloop's three native screens relies on this for error and empty states.
  • A keyed :for creates and keys rows correctly, and their handlers are live — :for sends createNode and then an updateNode carrying patch.events, which is exactly what :if does not do.

So the machinery for "node arrives later, handler attaches to it" exists and is used on the list path. :if is not reaching it.

Reproducing

# wildloop, on main
bun test tests/unit/native-notifications-screen.test.ts

'puts no :if on an interactive node' is a source guard standing in for the fix:

const interactive = SOURCE.match(/<(?:Button|Pressable|TouchableOpacity)[^>]*:if=/g)
expect(interactive).toBeNull()

A minimal reproduction is a screen with one Button behind an :if on a signal that starts false, and a test that flips the signal and dispatches the handler id the host was given.

What would be enough

Either of:

  1. Make :if use the same path :for already does — remove the node when false, createNode it when true, and register its handler against the new id. The protocol supports this; the list boundary proves it.
  2. Refuse it at compile time. If :if on an interactive node is not going to work, a compiler diagnostic naming the file and the node would have cost minutes instead of an afternoon. runNativeCompile already emits dropped-subtree diagnostics, so there is somewhere to put it.

(1) is obviously better. (2) is worth having anyway for whatever else turns out to be unsupported — a silent drop is the expensive kind of failure, and this is the second one of those in this codebase's native work (the first was useDebounceFn, declared in browser-auto-imports.d.ts and not injected into a client block, which threw at hydration and took a whole page's setup down).

Filed from wildloop's native promotion work for stacksjs/stx#1992. Three screens in: resources/native/ManualActivity.stx, Records.stx, Notifications.stx, Leaderboard.stx.

9 days ago

Narrowed it, with a throwaway screen carrying three buttons — one with no :if, one whose condition is true at mount, and one flipped to true in onMount:

always:         ran = true   ["always-ran"]
true-at-mount:  ran = true   ["mounted-ran"]
flipped-later:  ran = false  []

So the title overstates it. :if on an interactive node is fine when the condition holds at the first render — the node is created with the document and its handler is live. The failure is specific to a condition that turns true after that first render: the node is already in the document, nothing re-creates it, and the handler the host holds for it never fires. In all three cases a handler id is found, which is why this reads as a working button.

That also explains why it survived three screens before biting: every other :if in wildloop's native screens is either on a Text node or on a condition that is already settled when the screen mounts. The one that broke — :if="unread() > 0" on a Button — is false at mount by definition, because the data has not arrived yet.

Which sharpens what a fix needs to cover: it is the update path for :if, not the create path. :for does the same thing correctly on the same protocol (createNode, then updateNode carrying patch.events), so the shape of the answer already exists in the renderer.

The repro screen is four lines of template if it is useful:

<Text testID="probe-log" :text="log()" />
<Button testID="always" @click="always()">Always</Button>
<Button :if="onAtMount()" testID="true-at-mount" @click="mounted()">True at mount</Button>
<Button :if="flipped()" testID="flipped-later" @click="later()">Flipped later</Button>

with flipped.set(true) in onMount, driven through the bundle the way tests/fixtures/nativeScreen.ts does.

9 days ago

Verified on current main at 87af825 (fix(native): hydrate revealed if handlers in the same batch). The focused native regression in packages/stx/test/native/native-conditional-toggle.test.ts now covers a Button with :if initially false: after the signal flips, the runtime emits the node and native handler in the same turn, and dispatch reaches the handler. The full native suite is green: 306 passed, 0 failed. Closing as fixed.

Sign in to comment on this issue.