also looking at this
fix(sdk): restore craftVoiceAction — the event map is cross-platform
#135
3 files
+96
-12
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -1471,6 +1471,29 @@ export interface CraftWatchUserInfoEvent extends CustomEvent { | ||
| 1471 | 1471 | detail: Record<string, any>; |
| 1472 | 1472 | } |
| 1473 | 1473 | |
| 1474 | /** | |
| 1475 | * A registered voice action being spoken. **Android only** — | |
| 1476 | * `craft.voice.register()` has no iOS counterpart, and the iOS bridge never | |
| 1477 | * dispatches this. | |
| 1478 | */ | |
| 1479 | export interface CraftVoiceActionEvent extends CustomEvent { | |
| 1480 | detail: { | |
| 1481 | action: string; | |
| 1482 | data?: string; | |
| 1483 | }; | |
| 1484 | } | |
| 1485 | ||
| 1486 | /** | |
| 1487 | * Foreground/background transitions. **Android only** — dispatched by | |
| 1488 | * `startAppStateMonitoring`, which iOS answers through `getAppState` polling | |
| 1489 | * instead. | |
| 1490 | */ | |
| 1491 | export interface CraftAppStateChangeEvent extends CustomEvent { | |
| 1492 | detail: { | |
| 1493 | state: string; | |
| 1494 | }; | |
| 1495 | } | |
| 1496 | ||
| 1474 | 1497 | export interface CraftErrorEvent extends CustomEvent { |
| 1475 | 1498 | detail: CraftError; |
| 1476 | 1499 | } |
| @@ -1499,6 +1522,10 @@ declare global { | ||
| 1499 | 1522 | craftWatchReachability: CraftWatchReachabilityEvent; |
| 1500 | 1523 | craftWatchContext: CraftWatchContextEvent; |
| 1501 | 1524 | craftWatchUserInfo: CraftWatchUserInfoEvent; |
| 1525 | // Android-only. This file is the cross-platform surface, so it carries the | |
| 1526 | // union of what either bridge dispatches, not one platform's set. | |
| 1527 | craftVoiceAction: CraftVoiceActionEvent; | |
| 1528 | craftAppStateChange: CraftAppStateChangeEvent; | |
| 1502 | 1529 | craftError: CraftErrorEvent; |
| 1503 | 1530 | craftDeepLink: CraftDeepLinkEvent; |
| 1504 | 1531 | // Kept, and kept last, because nothing dispatches either one: the app |
| @@ -411,6 +411,9 @@ pub fn build(b: *std.Build) void { | ||
| 411 | 411 | ios_conformance_tests.root_module.addAnonymousImport("craft.d.ts", .{ |
| 412 | 412 | .root_source_file = b.path("../typescript/types/craft.d.ts"), |
| 413 | 413 | }); |
| 414 | ios_conformance_tests.root_module.addAnonymousImport("CraftBridge.kt", .{ | |
| 415 | .root_source_file = b.path("../android/templates/CraftBridge.kt.template"), | |
| 416 | }); | |
| 414 | 417 | ios_conformance_tests.root_module.addAnonymousImport("src/bridge_mobile_vision.zig", .{ |
| 415 | 418 | .root_source_file = b.path("src/bridge_mobile_vision.zig"), |
| 416 | 419 | }); |
| @@ -1579,18 +1579,73 @@ test "every recorded reply divergence is real, and still a divergence" { | ||
| 1579 | 1579 | // the spec's `sendToWeb` calls and `ios_events.Event`, and it was the only one |
| 1580 | 1580 | // nothing checked — so it had drifted in both directions at once. |
| 1581 | 1581 | // |
| 1582 | // It declared `craftVoiceAction`, which appears nowhere else in the repository: | |
| 1583 | // no dispatch, no subscription, no implementation. A listener written against | |
| 1584 | // it type-checks and never fires. | |
| 1585 | // | |
| 1586 | 1582 | // It omitted six events that do fire — `craftLocationUpdate`, |
| 1587 | 1583 | // `craftLocationError`, `craftNetworkChange`, `craftPushToken`, |
| 1588 | 1584 | // `craftNotificationResponse` and `craftWatchUserInfo` — so the most-used |
| 1589 | 1585 | // stream in the whole surface reached a page as an untyped `Event` whose |
| 1590 | 1586 | // `detail` was `any`. |
| 1587 | // | |
| 1588 | // ## This file is cross-platform, and the first version of this check was not | |
| 1589 | // | |
| 1590 | // `craft.d.ts` is the SDK's whole type surface, not iOS's. Checking it against | |
| 1591 | // the iOS spec alone got a live type deleted: `craftVoiceAction` is dispatched | |
| 1592 | // by `CraftBridge.kt`, and a scan that never opened a `.kt.template` reported | |
| 1593 | // it as fiction. The check that replaced it would then have rejected the fix, | |
| 1594 | // because a correct Android-only entry fails an iOS-only rule — a guard whose | |
| 1595 | // own error message argues for the bug. | |
| 1596 | // | |
| 1597 | // So the vocabulary here is the union of what either bridge dispatches. The | |
| 1598 | // Android scan below is the same shape as the Swift one: `sendEvent("craftX"` | |
| 1599 | // for the helper, plus the `CustomEvent('craftX'` literals the injected JS | |
| 1600 | // emits directly. Android's `window._craftXResolve` handles are deliberately | |
| 1601 | // not matched — those are promise plumbing, not events. | |
| 1591 | 1602 | // --------------------------------------------------------------------------- |
| 1592 | 1603 | |
| 1593 | 1604 | const sdk_types = @embedFile("craft.d.ts"); |
| 1605 | const android_spec = @embedFile("CraftBridge.kt"); | |
| 1606 | ||
| 1607 | /// Every event `CraftBridge.kt` dispatches to the page. | |
| 1608 | fn collectAndroidDispatchedEvents(allocator: std.mem.Allocator) !std.StringHashMap(void) { | |
| 1609 | var set = std.StringHashMap(void).init(allocator); | |
| 1610 | errdefer set.deinit(); | |
| 1611 | ||
| 1612 | const Needle = struct { text: []const u8, quote: u8 }; | |
| 1613 | const needles = [_]Needle{ | |
| 1614 | .{ .text = "sendEvent(\"", .quote = '"' }, | |
| 1615 | .{ .text = "CustomEvent('", .quote = '\'' }, | |
| 1616 | }; | |
| 1617 | ||
| 1618 | for (needles) |needle| { | |
| 1619 | var search: usize = 0; | |
| 1620 | while (std.mem.indexOfPos(u8, android_spec, search, needle.text)) |at| { | |
| 1621 | const start = at + needle.text.len; | |
| 1622 | const end = std.mem.indexOfScalarPos(u8, android_spec, start, needle.quote) orelse break; | |
| 1623 | search = end; | |
| 1624 | ||
| 1625 | const name = android_spec[start..end]; | |
| 1626 | if (!std.mem.startsWith(u8, name, "craft")) continue; | |
| 1627 | var ok = true; | |
| 1628 | for (name) |c| { | |
| 1629 | if (!std.ascii.isAlphanumeric(c)) ok = false; | |
| 1630 | } | |
| 1631 | if (ok) try set.put(name, {}); | |
| 1632 | } | |
| 1633 | } | |
| 1634 | return set; | |
| 1635 | } | |
| 1636 | ||
| 1637 | /// What either bridge dispatches — the vocabulary `craft.d.ts` describes. | |
| 1638 | fn collectAllDispatchedEvents(allocator: std.mem.Allocator) !std.StringHashMap(void) { | |
| 1639 | var set = try collectSpecDispatchedEvents(allocator); | |
| 1640 | errdefer set.deinit(); | |
| 1641 | ||
| 1642 | var android = try collectAndroidDispatchedEvents(allocator); | |
| 1643 | defer android.deinit(); | |
| 1644 | ||
| 1645 | var it = android.keyIterator(); | |
| 1646 | while (it.next()) |name| try set.put(name.*, {}); | |
| 1647 | return set; | |
| 1648 | } | |
| 1594 | 1649 | |
| 1595 | 1650 | /// The keys of `interface WindowEventMap { … }`. |
| 1596 | 1651 | fn collectSdkEventMap(allocator: std.mem.Allocator) !std.StringHashMap(void) { |
| @@ -1613,17 +1668,16 @@ fn collectSdkEventMap(allocator: std.mem.Allocator) !std.StringHashMap(void) { | ||
| 1613 | 1668 | } |
| 1614 | 1669 | |
| 1615 | 1670 | test "the SDK's event map declares nothing that never fires" { |
| 1616 | // `craftVoiceAction` was the case: a name in this map and nowhere else in | |
| 1617 | // the repository. The map is the most authoritative-looking of the three | |
| 1618 | // statements of the vocabulary — it is what an editor autocompletes from — | |
| 1619 | // and it was the one with no check behind it. | |
| 1671 | // The map is the most authoritative-looking statement of the vocabulary — | |
| 1672 | // it is what an editor autocompletes from — and it was the one with no | |
| 1673 | // check behind it. | |
| 1620 | 1674 | var map = try collectSdkEventMap(testing.allocator); |
| 1621 | 1675 | defer map.deinit(); |
| 1622 | 1676 | |
| 1623 | 1677 | // Non-vacuity: a renamed interface would empty this set. |
| 1624 | try testing.expect(map.count() >= 19); | |
| 1678 | try testing.expect(map.count() >= 21); | |
| 1625 | 1679 | |
| 1626 | var dispatched = try collectSpecDispatchedEvents(testing.allocator); | |
| 1680 | var dispatched = try collectAllDispatchedEvents(testing.allocator); | |
| 1627 | 1681 | defer dispatched.deinit(); |
| 1628 | 1682 | |
| 1629 | 1683 | var it = map.keyIterator(); |
| @@ -1658,9 +1712,9 @@ test "the SDK's event map declares everything that does fire" { | ||
| 1658 | 1712 | var map = try collectSdkEventMap(testing.allocator); |
| 1659 | 1713 | defer map.deinit(); |
| 1660 | 1714 | |
| 1661 | var dispatched = try collectSpecDispatchedEvents(testing.allocator); | |
| 1715 | var dispatched = try collectAllDispatchedEvents(testing.allocator); | |
| 1662 | 1716 | defer dispatched.deinit(); |
| 1663 | try testing.expect(dispatched.count() >= 17); | |
| 1717 | try testing.expect(dispatched.count() >= 20); | |
| 1664 | 1718 | |
| 1665 | 1719 | var it = dispatched.keyIterator(); |
| 1666 | 1720 | while (it.next()) |name| { |