ReviewOS

also looking at this

craft-native/craft

fix(ios): hand a notification tap that launches the app to the page that asks for it

#255
Merged glennmichael123 wants to merge fix/ios-notification-tap-cold-start into main
5 files +194 -9
packages/ios/src/index.test.tsmodified+27-0
Changes to packages/ios/src/index.test.ts
@@ -349,6 +349,33 @@ describe('Craft iOS builder', () => {
349349 expect(swift).toContain('if !wasConnected, self?.isConnected == true {')
350350 })
351351
352 it('holds a notification tap that launches the app until the page is ready', async () => {
353 // A tap on a killed app reaches the app delegate before SwiftUI has built
354 // the Coordinator. It was posted through NotificationCenter, which keeps
355 // nothing for an observer that does not exist yet, so the tap was lost.
356 // CraftEventManager exists from process start and holds it, as it already
357 // does for a home-screen shortcut.
358 const output = mkdtempSync(join(tmpdir(), 'craft-ios-notification-tap-'))
359 await init({ name: 'WildLoop', bundleId: 'org.wildloop.app', output })
360 const swift = readFileSync(join(output, 'Sources', 'WildLoopApp.swift'), 'utf8')
361
362 const didReceive = swift.slice(
363 swift.indexOf('didReceive response: UNNotificationResponse'),
364 swift.indexOf('willPresent notification: UNNotification'),
365 )
366 expect(didReceive).toContain('CraftEventManager.shared.handleNotificationResponse(')
367 expect(didReceive).not.toContain('NotificationCenter.default.post(')
368
369 // And only that path: a Coordinator still observing a NotificationCenter
370 // post would deliver a warm tap twice.
371 expect(swift).not.toContain('name: .craftNotificationResponse')
372 expect(swift).not.toContain('func receiveNotificationResponse(')
373
374 // It goes through the manager's buffer rather than straight to the page.
375 const handle = swift.slice(swift.indexOf('func handleNotificationResponse('))
376 expect(handle.slice(0, handle.indexOf('\n }\n'))).toContain('sendToWeb("craftNotificationResponse"')
377 })
378
352379 it('settles calendar callbacks only after a real EventKit operation', async () => {
353380 const output = mkdtempSync(join(tmpdir(), 'craft-ios-calendar-'))
354381 await init({
packages/ios/src/page-script.test.tsmodified+52-0
Changes to packages/ios/src/page-script.test.ts
@@ -91,6 +91,9 @@ function loadPage(beforeInject?: (page: Record<string, any>) => void, seed = SEE
9191 // What DeepLinkManager.dispatchDeepLink evaluates, once the script has run.
9292 link: (url: string, initial: boolean) =>
9393 page.dispatchEvent(new CustomEvent('craftDeepLink', { detail: { url, initial } })),
94 // What CraftEventManager.handleNotificationResponse evaluates.
95 tap: (detail: unknown) =>
96 page.dispatchEvent(new CustomEvent('craftNotificationResponse', { detail })),
9497 }
9598}
9699
@@ -201,6 +204,55 @@ describe('the injected iOS page script', () => {
201204 })
202205 }
203206
207 // A notification tap on a cold launch is flushed the moment the bridge is
208 // ready, and a page that wires its listener after its router hydrates was
209 // not listening yet: "Try it" opened the home screen instead. Held the way
210 // deep links are (#198), and handed to the first subscriber.
211 it('hands a tap that arrived before anyone subscribed to the first subscriber', async () => {
212 const page = loadPage()
213 page.tap({ screen: 'plant-id' })
214
215 const seen: unknown[] = []
216 page.craft.notifications.onTap((detail: unknown) => seen.push(detail))
217 await tick()
218
219 expect(seen).toEqual([{ screen: 'plant-id' }])
220 })
221
222 it('keeps onTap on the notifications object the page ends up with', () => {
223 // installCraftMobileContract replaces craft.notifications after the
224 // replay attaches onTap, and onTap survives only because it copies own
225 // properties across. Reorder those two blocks and it would vanish with
226 // nothing else failing, so this reads the final object.
227 const page = loadPage()
228 expect(typeof page.craft.notifications.onTap).toBe('function')
229 expect(typeof page.craft.notifications.onTap(() => {})).toBe('function')
230 // And the contract's own additions are still there beside it.
231 expect(typeof page.craft.notifications.show).toBe('function')
232 })
233
234 it('delivers a tap once, live, to a page already subscribed', async () => {
235 const page = loadPage()
236 const seen: unknown[] = []
237 page.craft.notifications.onTap((detail: unknown) => seen.push(detail))
238 await tick()
239
240 page.tap({ screen: 'recap' })
241 expect(seen).toEqual([{ screen: 'recap' }])
242 })
243
244 it('drops held taps for a subscriber that unsubscribed in the same tick', async () => {
245 const page = loadPage()
246 page.tap({ screen: 'plant-id' })
247
248 const seen: unknown[] = []
249 const unsubscribe = page.craft.notifications.onTap((detail: unknown) => seen.push(detail))
250 unsubscribe()
251 await tick()
252
253 expect(seen).toEqual([])
254 })
255
204256 // #198 and #215. The Android page script runs the same block; its own test
205257 // covers the rest of the contract.
206258 const tick = () => new Promise(resolve => setTimeout(resolve, 5))
packages/ios/templates/CraftApp.swiftmodified+65-8
Changes to packages/ios/templates/CraftApp.swift
@@ -34,7 +34,6 @@ import ActivityKit
3434extension Notification.Name {
3535 static let craftPushToken = Notification.Name("craftPushToken")
3636 static let craftPushRegistrationError = Notification.Name("craftPushRegistrationError")
37 static let craftNotificationResponse = Notification.Name("craftNotificationResponse")
3837}
3938
4039/// Which failed page loads mean the remote origin is out of reach (#252).
@@ -104,7 +103,14 @@ final class CraftAppDelegate: NSObject, UIApplicationDelegate, UNUserNotificatio
104103 }
105104
106105 func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) {
107 NotificationCenter.default.post(name: .craftNotificationResponse, object: response.notification.request.content.userInfo)
106 // Through the manager, not NotificationCenter. A tap that launches a
107 // killed app arrives here before SwiftUI has built the Coordinator, and
108 // NotificationCenter keeps nothing for an observer that does not exist
109 // yet — so the tap was dropped, and the page opened wherever it opens
110 // rather than where the notification pointed. The manager exists from
111 // process start and holds the tap until the page is ready, the way it
112 // already does for a home-screen shortcut.
113 CraftEventManager.shared.handleNotificationResponse(response.notification.request.content.userInfo)
108114 completionHandler()
109115 }
110116
@@ -144,6 +150,20 @@ class CraftEventManager {
144150 sendToWeb("craftShortcut", data: ["type": shortcut.type])
145151 }
146152
153 /// A tap on a notification, carrying the payload it was sent with.
154 ///
155 /// Keys that are not strings are dropped rather than failing the whole
156 /// event: an APNs payload's keys always are, and the page could not index
157 /// by anything else anyway.
158 func handleNotificationResponse(_ userInfo: [AnyHashable: Any]) {
159 var data: [String: Any] = [:]
160 for (key, value) in userInfo {
161 guard let key = key as? String else { continue }
162 data[key] = value
163 }
164 sendToWeb("craftNotificationResponse", data: data)
165 }
166
147167 func handleSiriActivity(_ activity: NSUserActivity) -> Bool {
148168 let prefix = "\(Bundle.main.bundleIdentifier ?? "{{BUNDLE_ID}}")."
149169 guard activity.activityType.hasPrefix(prefix) else { return false }
@@ -635,7 +655,6 @@ struct CraftWebView: UIViewRepresentable {
635655 }
636656 NotificationCenter.default.addObserver(self, selector: #selector(receivePushToken(_:)), name: .craftPushToken, object: nil)
637657 NotificationCenter.default.addObserver(self, selector: #selector(receivePushRegistrationError(_:)), name: .craftPushRegistrationError, object: nil)
638 NotificationCenter.default.addObserver(self, selector: #selector(receiveNotificationResponse(_:)), name: .craftNotificationResponse, object: nil)
639658 NotificationCenter.default.addObserver(self, selector: #selector(appWillEnterForeground(_:)), name: UIApplication.willEnterForegroundNotification, object: nil)
640659 setupNetworkMonitoring()
641660 }
@@ -3090,6 +3109,49 @@ struct CraftWebView: UIViewRepresentable {
30903109 };
30913110 })(window.craft);
30923111
3112 // A notification tap that arrived before anything subscribed.
3113 //
3114 // On a cold launch the tap is flushed the moment the bridge is
3115 // ready, and a page that wires its listener once its router has
3116 // hydrated — which is most single-page apps — was not listening
3117 // yet, so "open the thing this notification is about" opened the
3118 // home screen instead. Deep links hit the same wall (#198) and
3119 // are held the same way: state on window, so a second injection
3120 // into the same page does not start a fresh buffer, and the first
3121 // subscriber is handed whatever it missed.
3122 (function installNotificationTapReplay(craft) {
3123 var replay = window.__craftNotificationTapReplay;
3124 if (!replay) {
3125 replay = window.__craftNotificationTapReplay = {undelivered: [], subscribed: false};
3126 window.addEventListener('craftNotificationResponse', function(e) {
3127 if (!replay.subscribed) replay.undelivered.push(e.detail);
3128 });
3129 }
3130 craft._subscribeNotificationTaps = function(callback) {
3131 var active = true;
3132 var listener = function(e) { callback(e.detail); };
3133 window.addEventListener('craftNotificationResponse', listener);
3134 if (!replay.subscribed) {
3135 replay.subscribed = true;
3136 setTimeout(function() {
3137 var pending = replay.undelivered;
3138 replay.undelivered = [];
3139 if (!active) return;
3140 pending.forEach(function(detail) { callback(detail); });
3141 }, 0);
3142 }
3143 return function() {
3144 active = false;
3145 window.removeEventListener('craftNotificationResponse', listener);
3146 };
3147 };
3148 if (craft.notifications) {
3149 craft.notifications.onTap = function(callback) {
3150 return craft._subscribeNotificationTaps(callback);
3151 };
3152 }
3153 })(window.craft);
3154
30933155 (function installCraftMobileContract(craft) {
30943156 var legacyShare = craft.share.bind(craft);
30953157 var legacyOpenCamera = craft.openCamera.bind(craft);
@@ -3590,11 +3652,6 @@ struct CraftWebView: UIViewRepresentable {
35903652 rejectCallback(claimed, error: message, code: "PUSH_REGISTRATION_ERROR")
35913653 }
35923654
3593 @objc private func receiveNotificationResponse(_ notification: Notification) {
3594 let data = notification.object as? [String: Any] ?? [:]
3595 sendToWeb("craftNotificationResponse", data: data)
3596 }
3597
35983655 // MARK: - Secure Storage (Keychain)
35993656 private func secureStore(key: String, value: String) -> Bool {
36003657 let data = value.data(using: .utf8)!
packages/typescript/src/__tests__/mobile.test.tsmodified+37-1
Changes to packages/typescript/src/__tests__/mobile.test.ts
@@ -24,7 +24,7 @@ import type {
2424 LiveActivityHandle,
2525 LiveActivityOptions,
2626} from '../api/mobile'
27import { normalizeDeepLinkURL, watchConnectivity } from '../api/mobile'
27import { normalizeDeepLinkURL, pushNotifications, watchConnectivity } from '../api/mobile'
2828
2929describe('Mobile deep links', () => {
3030 it('normalizes native payloads to the public string contract', () => {
@@ -34,6 +34,42 @@ describe('Mobile deep links', () => {
3434 })
3535})
3636
37describe('Mobile notification taps', () => {
38 // A tap that launched the app is flushed before a hydrating page subscribes.
39 // The bridge holds it for the first subscriber through notifications.onTap,
40 // and the SDK has to go through that rather than a bare event listener, or
41 // the hold does nothing for anyone using the SDK.
42 it('subscribes through the bridge replay when the bridge offers one', () => {
43 const previousWindow = (globalThis as any).window
44 const unsubscribe = () => {}
45 let handed: ((detail: unknown) => void) | undefined
46 ;(globalThis as any).window = {
47 craft: {
48 notifications: {
49 onTap: (callback: (detail: unknown) => void) => {
50 handed = callback
51 return unsubscribe
52 },
53 },
54 },
55 }
56
57 try {
58 const seen: unknown[] = []
59 const returned = pushNotifications.onNotification(data => seen.push(data))
60 expect(returned).toBe(unsubscribe)
61
62 // The replay hands over whatever it held; the SDK passes it through.
63 handed?.({ screen: 'plant-id' })
64 expect(seen).toEqual([{ screen: 'plant-id' }])
65 }
66 finally {
67 if (previousWindow === undefined) delete (globalThis as any).window
68 else (globalThis as any).window = previousWindow
69 }
70 })
71})
72
3773describe('Mobile Android bridge promises', () => {
3874 it('reads the typed watch reachability envelope', async () => {
3975 const previousWindow = (globalThis as any).window
packages/typescript/src/api/mobile.tsmodified+13-0
Changes to packages/typescript/src/api/mobile.ts
@@ -1360,7 +1360,20 @@ export const pushNotifications = {
13601360 onToken(callback: (token: string) => void): () => void {
13611361 return onCraftEvent('craftPushToken', detail => callback(String(detail.token ?? '')))
13621362 },
1363 /**
1364 * A tap on a notification, with the payload it was sent with.
1365 *
1366 * Goes through the bridge's replay where there is one, so a tap that
1367 * launched the app still reaches a page that subscribes once it has
1368 * hydrated — rather than only one listening before the bridge was ready.
1369 */
13631370 onNotification(callback: (data: Record<string, unknown>) => void): () => void {
1371 const craft = getCraftRoot()
1372 if (craft?.notifications?.onTap) {
1373 return craft.notifications.onTap((detail: unknown) => {
1374 callback((detail ?? {}) as Record<string, unknown>)
1375 }) ?? (() => {})
1376 }
13641377 return onCraftEvent('craftNotificationResponse', detail => callback(detail))
13651378 }
13661379}