ReviewOS

also looking at this

craft-native/craft

fix(ios): let share() settle the promise its native side answers

#208
Merged glennmichael123 wants to merge fix/share-settles-promise into main
7 files +302 -22
10 days ago

Part of #203. This PR is the iOS half; Android comes next.

What was wrong

craft.share(text) posted {action: 'share', text} with no callbackId. The Swift case for share was already written to answer:

  • true or false depending on whether the person finished sharing
  • CAPABILITY_DISABLED when enableShare is off
  • INVALID_ARGUMENT when there is nothing to share
  • SHARE_ERROR when the share sheet reports an error

But resolveCallback and rejectCallback both begin guard let id = callbackId else { return }, so every one of those answers was dropped. The page got undefined back, and awaiting undefined looks like success. An app with sharing switched off would call share, hear nothing and carry on.

The fix

Both entry points, craft.share(text) and craft.share.share(options), now go through one _share helper. It posts a callbackId and returns the promise, the same way openCamera does. Swift and Zig needed no changes. Both already reply to cb_N: Swift through resolveCallback, Zig through the request id CraftZigRuntime.offer passes down.

One deliberate difference: share.share(options) used to go through _invoke, which rejects after 30 seconds. It no longer does. The share sheet waits on a person, and anyone slower than 30 seconds to pick an app would have been told the share failed while the sheet was still open, and the real answer would then have been dropped. openCamera, pickImage and authenticate already skip that timeout for the same reason.

Tests

A static check for this whole class of bug. ios_conformance_test.zig now scans every postMessage(...) in the page script. It fails when a post has no callbackId for an action whose Swift case answers through one.

  • The scanner is checked first against known shapes, including {action: 'startAR', options: options || {}, callbackId: id}. A naive [^}]* scan misreads that {} as the end of the object.
  • On the old template it fails with: "the page posts share with no callbackId, and the spec answers it."

The scan found five more actions with the same problem: haptic, startListening, watchPosition, clearWatch and vibrate. Each needs its own look at what the page should get back, so they are recorded rather than fixed here and filed as #207. A second test fails if any of those five gets fixed but stays on the list, so the list can only shrink.

On a simulator. The iOS E2E suite gains share.disabled.rejects. The probe app now sets enableShare: false explicitly, and the case requires craft.share(...) to reject with CAPABILITY_DISABLED. Before the fix, the promise resolved with undefined. In the runtime leg, the check also requires Zig to dispatch share and to write its own refusal line for it, so the rejection is known to come from Zig's capability gate rather than from Swift.

ok   ios-shim — 5/5 cases, no zig
ok   ios-runtime — 5/5 cases, zig served clipboardRead, clipboardWrite, getCurrentPosition, getDeviceInfo, log, share

Verification

bun run test, bun run typecheck, bun run verify:packages, Pickier, the 34 protocol tests and both iOS E2E legs on an iOS 26.5 simulator.

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

Sign in to comment on this pull request.