ReviewOS

stacks/stx

The native testing kit's press() ignores link targets, so a test cannot see the host pre-empt a handler

#2147
Closed glennmichael123 opened this 6 hours ago ·
6 hours ago

press() dispatches a node's handler whatever link target that node carries, so a native screen test cannot observe the host rule that a link target pre-empts the handler. A test over a screen broken by that rule passes.

Where

packages/stx/src/native/testing.ts:697

const press = (node: NativeNode, event = 'onPress', nativeEvent: Record<string, unknown> = {}) => {
  if (!node) throw new Error(`nothing to ${event}`)
  if (!node.events[event]) throw new Error(`${node.type} has no ${event}`)
  send('EVENT', { handlerId: node.events[event], nativeEvent })
}

The host rule it is missing

A native host registers a link target for any Link, TouchableOpacity, TouchableHighlight or Pressable with a non-empty href or to and not disabled. On a tap it opens that target and returns, so the authored handler never runs. Both platforms do this:

  • iOS: packages/ios/templates/CraftNativeScreen.swift:11406 in viewPressed
  • Android: packages/android/templates/MainActivityNative.kt.template:6104 in setOnClickListener

Filed against the host as craft-native/craft#442, where the behaviour itself is the question. Whatever that resolves to, the kit should model it rather than model nothing, because today it models the one thing the host never does.

What that cost

stacksjs/hq.training#392. HQ's consent gate listed its legal documents as rows carrying both an href and an onPress, and only the handler opened them. The screen was broken on a device for every user, and this test passed over it the whole time:

phone.press(phone.find(byTest('agree-read-terms'))!)
await until(() => pages.length === 3)
expect(pages).toEqual([...])        // the browser opened each document
expect(phone.opened).toEqual([])    // and nothing navigated

The assertions are the right ones. expect(phone.opened).toEqual([]) is exactly the thing that was false on the device. It passed because press went straight to the handler, which is the code path the host had taken away. The bug was then found by hand on a simulator, and the regression test that catches it has to read node.props.href instead of pressing, which tests the authoring rather than the behaviour.

What good would look like

press on a node that carries a link target should do what the host does: record the navigation in opened and not dispatch the handler. Then the test above fails on the broken screen and passes on the fixed one, with no change to the test.

Worth keeping the predicate in one place rather than restating it, since it has three parts that must agree with the host: the node types, a non-empty href or to, and disabled. If craft-native/craft#442 lands on dispatching the handler instead, this becomes the same change in the other direction, and the kit should follow it rather than keep its own answer.

Found through @stacksjs/stx 0.2.x with Craft 0.0.420, iOS 26.5 simulator.

6 hours ago

Fixed on main in ebc846c.

Version: unreleased source after STX 0.2.784. This fix is not in the 0.2.784 npm packages; no release/tag was cut.

phone.press() now models the host's primary-tap decision in one place: Link/TouchableOpacity/TouchableHighlight/Pressable targets take precedence over the authored callback, using the first nonempty string href/to and trimming it. Navigation is recorded in opened, including handlerless links. Disabled taps and removed/replaced/ended node references stay inert. Other named native events retain their payload delivery.

opened records the host's requested target before platform routing; it does not fabricate a screen-side Linking API call. Three existing navigation tests now independently assert real host-tap outcomes and explicitly delivered runtime events. Their original runtime URL-normalization, cancellation and system-link assertions remain covered.

Maintained regressions: 24 cases across normal/minified bundles and iOS/Android VM flags, including the reported consent-row blind spot. The broken target-plus-handler row now fails the same business gate that accepts its handler-only fix. The original testing module fails 20 of these 24 cases; the corrected module passes them all.

Validation: 1,290 native tests passed / zero failures, 186 files, 15,723 assertions. Focused testing-kit/navigation/scoped-slot checks: 50 passed / 261 assertions. Typecheck, lint and formatting pass. Hosted CI for the new commit is separate and pending.

Current host behavior was checked against clean Craft 30b4613. Craft code was untouched. craft-native/craft#442 remains the host behavior discussion; this issue fixes the kit's fidelity to the existing rule.

Testing-kit API documentation.

Sign in to comment on this issue.