ReviewOS

also looking at this

craft-native/craft

fix(android): hand the launch link to the page that subscribes for it

#232
Merged glennmichael123 wants to merge fix/android-deep-link-replay into main
17 files +776 -136
9 days ago

Closes #215.

What was wrong

On Android, the link that launched the app went only to getInitialURL(). A page that subscribed with onLink never learned how it was opened, which is #198's failure on iOS. And every warm link replaced getInitialURL's answer: in MainActivity through setInitialURL, and on the Zig path through rememberFirst.

What it does now

The Android side now meets the contract iOS has had since #216.

Native

  • The launch link is queued until the page's bridge has been injected, then dispatched once with initial: true. The flush runs in the injection's completion callback, after the page's replay listener and the native deliverer exist.
  • Later links are initial: false and leave getInitialURL alone.
  • Zig takes the flag over JNI: nativeDispatchDeepLink is now (Ljava/lang/String;Z)Z, and the descriptor is pinned in a Zig test.
  • A relaunch from Recents (FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY) is not treated as opened by its old link.
  • A recreated activity restores the launch link from saved instance state, not from getIntent(), which onNewIntent may have replaced with a warm link. The link is queued again for the new WebView's fresh document.

Page

  • The replay block from iOS: links wait for the first subscriber, and onLink/onDeepLink return an unsubscribe.
  • Its state lives on window, because Android can run the script twice in one document.

A fix for both platforms

A craftReady handler runs before native has dispatched anything. So when it called getInitialURL() and then onLink(), the claim found nothing to remove, and the launch link was delivered a second time. That was already true on iOS, and porting the block unchanged would have made it true on Android, where that pattern currently gets the link once.

getInitialURL now sets a sticky claim that also stops the launch link reaching live subscribers. The block is identical in CraftApp.swift.

A page that calls getInitialURL() only after an await still gets the launch link from both, as on iOS since #216. Such a page can now tell the two apart:

  • craft.d.ts says so;
  • the SDK's deepLinks.onLink passes { initial } as a second argument, where it used to drop it;
  • examples/basic-app.ts skips initial in its handler.

Tests that fail without the fix

  • packages/android/src/page-script.test.ts (new) runs the real injected script under Bun.
    • Against main's template, 5 of 9 fail: subscribe-late, the unsubscribe return, the craftReady case, the shared replay and unsubscribe-in-the-same-tick.
    • Removing the claim flag fails the craftReady case.
    • Removing the window state fails the reinjection case.
  • packages/ios/src/page-script.test.ts gains deep-link cases. The craftReady one fails against main.
  • packages/android/src/index.test.ts pins the generated Kotlin: the launch link queued rather than dropped, flushed after bridgeReady, initial in the detail, no first-link overwrite, save and restore on recreation, and the JNI signature.
  • E2E: each Android leg now cold-starts the app twice through crafte2eprobe://… with adb shell am start -W -a VIEW -d '<link>' <package>. That resolves through the generated intent filter, and -W must report LaunchState: COLD. The two modes and their judgement are iOS's own: subscribe-only must get the link once, marked initial; getInitialURL plus subscribe must get it once, from getInitialURL. The report is read from logcat. The runtime leg also requires Zig to register, and not decline, on each cold start.

Not verified locally

This machine has no Android toolchain. The Kotlin compile (android-builder), the Zig JNI path and the emulator runs are only checked by this PR's CI.

Locally: bun run test, bun run typecheck, bun run verify:packages, Pickier, the android/ios/typescript package tests, 55 protocol tests, and the Swift template typecheck.

Filed along the way

  • #228: bridge injection is not idempotent, and a stale injection callback can mark the wrong page ready.
  • #229: the Recents relaunch re-fires the launch shortcut. Its deep-link half is fixed here.
  • #230: compare JNI descriptors with the Kotlin external fun types, not just names.
  • #231: CraftNative.deliver drops scripts silently with no deliverer.
  • A comment on #209: enableDeepLinks is not enforced on Android.

17 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.