also looking at this
fix(sdk): the event map typed one event that never fires and missed six that do
#133
18 files
+562
-163
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.
| @@ -1145,13 +1145,33 @@ export interface PickedFile { | ||
| 1145 | 1145 | mimeType?: string; |
| 1146 | 1146 | } |
| 1147 | 1147 | |
| 1148 | /** | |
| 1149 | * A location fix, exactly as the bridge sends it. | |
| 1150 | * | |
| 1151 | * All eight fields are always present. The native dictionary is built | |
| 1152 | * unconditionally from `CLLocation`'s non-optional properties, so there is no | |
| 1153 | * path that omits one — and this type previously declared six of them, three | |
| 1154 | * of those optional, which left `altitudeAccuracy` and `timestamp` unreachable | |
| 1155 | * from TypeScript even though every fix carries them. | |
| 1156 | * | |
| 1157 | * `heading` and `speed` are `-1` when the device has no fix for them, which is | |
| 1158 | * CoreLocation's own sentinel rather than an absent key. Android has no | |
| 1159 | * geolocation surface, so nothing else fills this in. | |
| 1160 | */ | |
| 1148 | 1161 | export interface Position { |
| 1149 | 1162 | latitude: number; |
| 1150 | 1163 | longitude: number; |
| 1164 | /** Horizontal accuracy in metres. */ | |
| 1151 | 1165 | accuracy: number; |
| 1152 | altitude?: number; | |
| 1153 | speed?: number; | |
| 1154 | heading?: number; | |
| 1166 | altitude: number; | |
| 1167 | /** Vertical accuracy in metres. */ | |
| 1168 | altitudeAccuracy: number; | |
| 1169 | /** Course over ground in degrees, or `-1` when unavailable. */ | |
| 1170 | heading: number; | |
| 1171 | /** Metres per second, or `-1` when unavailable. */ | |
| 1172 | speed: number; | |
| 1173 | /** Milliseconds since the Unix epoch. */ | |
| 1174 | timestamp: number; | |
| 1155 | 1175 | } |
| 1156 | 1176 | |
| 1157 | 1177 | export interface Contact { |
| @@ -1378,12 +1398,29 @@ export interface CraftARPlaneEvent extends CustomEvent { | ||
| 1378 | 1398 | detail: ARPlaneEvent; |
| 1379 | 1399 | } |
| 1380 | 1400 | |
| 1401 | /** | |
| 1402 | * A home-screen quick action being tapped. | |
| 1403 | * | |
| 1404 | * **Never dispatched today.** `craft.shortcuts.set()` really does install the | |
| 1405 | * items and they appear in the long-press menu, but no template implements | |
| 1406 | * `application(_:performActionFor:completionHandler:)` or a scene-delegate | |
| 1407 | * equivalent, so a tap launches the app and stops there. A listener registered | |
| 1408 | * for this is never called. | |
| 1409 | */ | |
| 1381 | 1410 | export interface CraftShortcutEvent extends CustomEvent { |
| 1382 | 1411 | detail: { |
| 1383 | 1412 | type: string; |
| 1384 | 1413 | }; |
| 1385 | 1414 | } |
| 1386 | 1415 | |
| 1416 | /** | |
| 1417 | * A donated Siri shortcut being invoked. | |
| 1418 | * | |
| 1419 | * **Never dispatched today.** `craft.siri.register()` donates an | |
| 1420 | * `NSUserActivity`, but the only `onContinueUserActivity` in the template is | |
| 1421 | * bound to `NSUserActivityTypeBrowsingWeb` and routes to the deep-link | |
| 1422 | * handler, so invoking a donated shortcut relaunches the app and is dropped. | |
| 1423 | */ | |
| 1387 | 1424 | export interface CraftSiriShortcutEvent extends CustomEvent { |
| 1388 | 1425 | detail: SiriInvocationEvent; |
| 1389 | 1426 | } |
| @@ -1402,13 +1439,38 @@ export interface CraftWatchContextEvent extends CustomEvent { | ||
| 1402 | 1439 | detail: Record<string, any>; |
| 1403 | 1440 | } |
| 1404 | 1441 | |
| 1405 | export interface CraftVoiceActionEvent extends CustomEvent { | |
| 1442 | export interface CraftLocationUpdateEvent extends CustomEvent { | |
| 1443 | detail: Position; | |
| 1444 | } | |
| 1445 | ||
| 1446 | export interface CraftLocationErrorEvent extends CustomEvent { | |
| 1406 | 1447 | detail: { |
| 1407 | action: string; | |
| 1408 | data?: string; | |
| 1448 | message: string; | |
| 1409 | 1449 | }; |
| 1410 | 1450 | } |
| 1411 | 1451 | |
| 1452 | export interface CraftNetworkChangeEvent extends CustomEvent { | |
| 1453 | detail: NetworkStatus; | |
| 1454 | } | |
| 1455 | ||
| 1456 | export interface CraftPushTokenEvent extends CustomEvent { | |
| 1457 | detail: { | |
| 1458 | token: string; | |
| 1459 | }; | |
| 1460 | } | |
| 1461 | ||
| 1462 | /** | |
| 1463 | * The `userInfo` of the notification the user acted on, forwarded verbatim. | |
| 1464 | * The bridge does not reshape it, so its keys are whatever was scheduled. | |
| 1465 | */ | |
| 1466 | export interface CraftNotificationResponseEvent extends CustomEvent { | |
| 1467 | detail: Record<string, any>; | |
| 1468 | } | |
| 1469 | ||
| 1470 | export interface CraftWatchUserInfoEvent extends CustomEvent { | |
| 1471 | detail: Record<string, any>; | |
| 1472 | } | |
| 1473 | ||
| 1412 | 1474 | export interface CraftErrorEvent extends CustomEvent { |
| 1413 | 1475 | detail: CraftError; |
| 1414 | 1476 | } |
| @@ -1428,14 +1490,24 @@ declare global { | ||
| 1428 | 1490 | craftMotionUpdate: CraftMotionUpdateEvent; |
| 1429 | 1491 | craftBluetoothDevice: CraftBluetoothDeviceEvent; |
| 1430 | 1492 | craftARPlane: CraftARPlaneEvent; |
| 1431 | craftShortcut: CraftShortcutEvent; | |
| 1432 | craftSiriShortcut: CraftSiriShortcutEvent; | |
| 1493 | craftLocationUpdate: CraftLocationUpdateEvent; | |
| 1494 | craftLocationError: CraftLocationErrorEvent; | |
| 1495 | craftNetworkChange: CraftNetworkChangeEvent; | |
| 1496 | craftPushToken: CraftPushTokenEvent; | |
| 1497 | craftNotificationResponse: CraftNotificationResponseEvent; | |
| 1433 | 1498 | craftWatchMessage: CraftWatchMessageEvent; |
| 1434 | 1499 | craftWatchReachability: CraftWatchReachabilityEvent; |
| 1435 | 1500 | craftWatchContext: CraftWatchContextEvent; |
| 1436 | craftVoiceAction: CraftVoiceActionEvent; | |
| 1501 | craftWatchUserInfo: CraftWatchUserInfoEvent; | |
| 1437 | 1502 | craftError: CraftErrorEvent; |
| 1438 | 1503 | craftDeepLink: CraftDeepLinkEvent; |
| 1504 | // Kept, and kept last, because nothing dispatches either one: the app | |
| 1505 | // installs the shortcuts and donates the activities, and no template | |
| 1506 | // implements the callback that would deliver a tap. Typed so the listener | |
| 1507 | // an app writes today compiles against the shape it will receive if that | |
| 1508 | // lands; see the note on each interface. | |
| 1509 | craftShortcut: CraftShortcutEvent; | |
| 1510 | craftSiriShortcut: CraftSiriShortcutEvent; | |
| 1439 | 1511 | } |
| 1440 | 1512 | } |
| 1441 | 1513 | |
| @@ -408,6 +408,9 @@ pub fn build(b: *std.Build) void { | ||
| 408 | 408 | ios_conformance_tests.root_module.addAnonymousImport("src/ios_events.zig", .{ |
| 409 | 409 | .root_source_file = b.path("src/ios_events.zig"), |
| 410 | 410 | }); |
| 411 | ios_conformance_tests.root_module.addAnonymousImport("craft.d.ts", .{ | |
| 412 | .root_source_file = b.path("../typescript/types/craft.d.ts"), | |
| 413 | }); | |
| 411 | 414 | ios_conformance_tests.root_module.addAnonymousImport("src/bridge_mobile_vision.zig", .{ |
| 412 | 415 | .root_source_file = b.path("src/bridge_mobile_vision.zig"), |
| 413 | 416 | }); |
| @@ -1569,3 +1569,108 @@ test "every recorded reply divergence is real, and still a divergence" { | ||
| 1569 | 1569 | |
| 1570 | 1570 | try testing.expect(deliberate_reply_divergences.len >= 3); |
| 1571 | 1571 | } |
| 1572 | ||
| 1573 | // --------------------------------------------------------------------------- | |
| 1574 | // The SDK's event map | |
| 1575 | // | |
| 1576 | // `craft.d.ts` augments `WindowEventMap`, which is what gives | |
| 1577 | // `window.addEventListener('craftLocationUpdate', …)` a typed `detail` instead | |
| 1578 | // of a bare `Event`. It is a third statement of the event vocabulary, after | |
| 1579 | // the spec's `sendToWeb` calls and `ios_events.Event`, and it was the only one | |
| 1580 | // nothing checked — so it had drifted in both directions at once. | |
| 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 | // It omitted six events that do fire — `craftLocationUpdate`, | |
| 1587 | // `craftLocationError`, `craftNetworkChange`, `craftPushToken`, | |
| 1588 | // `craftNotificationResponse` and `craftWatchUserInfo` — so the most-used | |
| 1589 | // stream in the whole surface reached a page as an untyped `Event` whose | |
| 1590 | // `detail` was `any`. | |
| 1591 | // --------------------------------------------------------------------------- | |
| 1592 | ||
| 1593 | const sdk_types = @embedFile("craft.d.ts"); | |
| 1594 | ||
| 1595 | /// The keys of `interface WindowEventMap { … }`. | |
| 1596 | fn collectSdkEventMap(allocator: std.mem.Allocator) !std.StringHashMap(void) { | |
| 1597 | var set = std.StringHashMap(void).init(allocator); | |
| 1598 | errdefer set.deinit(); | |
| 1599 | ||
| 1600 | const start = std.mem.indexOf(u8, sdk_types, "interface WindowEventMap {") orelse | |
| 1601 | return error.EventMapNotFound; | |
| 1602 | const body = sdk_types[start..]; | |
| 1603 | const end = std.mem.indexOf(u8, body, "\n }") orelse return error.EventMapNotFound; | |
| 1604 | ||
| 1605 | var it = std.mem.splitScalar(u8, body[0..end], '\n'); | |
| 1606 | while (it.next()) |line| { | |
| 1607 | const trimmed = std.mem.trim(u8, line, " \t\r"); | |
| 1608 | if (!std.mem.startsWith(u8, trimmed, "craft")) continue; | |
| 1609 | const colon = std.mem.indexOfScalar(u8, trimmed, ':') orelse continue; | |
| 1610 | try set.put(trimmed[0..colon], {}); | |
| 1611 | } | |
| 1612 | return set; | |
| 1613 | } | |
| 1614 | ||
| 1615 | 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. | |
| 1620 | var map = try collectSdkEventMap(testing.allocator); | |
| 1621 | defer map.deinit(); | |
| 1622 | ||
| 1623 | // Non-vacuity: a renamed interface would empty this set. | |
| 1624 | try testing.expect(map.count() >= 19); | |
| 1625 | ||
| 1626 | var dispatched = try collectSpecDispatchedEvents(testing.allocator); | |
| 1627 | defer dispatched.deinit(); | |
| 1628 | ||
| 1629 | var it = map.keyIterator(); | |
| 1630 | while (it.next()) |name| { | |
| 1631 | if (dispatched.contains(name.*)) continue; | |
| 1632 | ||
| 1633 | // The two recorded dead subscriptions are typed on purpose: the page | |
| 1634 | // subscribes to them today, and a listener written now should compile | |
| 1635 | // against the shape it will receive when the missing half lands. Any | |
| 1636 | // other name here is a type with nothing behind it. | |
| 1637 | var recorded = false; | |
| 1638 | for (dead_subscriptions) |d| { | |
| 1639 | if (std.mem.eql(u8, d.event, name.*)) recorded = true; | |
| 1640 | } | |
| 1641 | if (recorded) continue; | |
| 1642 | ||
| 1643 | std.debug.print( | |
| 1644 | "craft.d.ts types `{s}` in WindowEventMap, and nothing dispatches it.\n" ++ | |
| 1645 | " A listener written against it type-checks and is never called.\n", | |
| 1646 | .{name.*}, | |
| 1647 | ); | |
| 1648 | return error.SdkTypesAnEventNothingEmits; | |
| 1649 | } | |
| 1650 | } | |
| 1651 | ||
| 1652 | test "the SDK's event map declares everything that does fire" { | |
| 1653 | // The other direction, and the one that had six holes in it. An event | |
| 1654 | // missing from the map is not a compile error for the page — the | |
| 1655 | // `addEventListener` overload falls back to `Event`, whose `detail` is | |
| 1656 | // `any` — so the cost is silent: no autocomplete, no shape, no check that | |
| 1657 | // the field being read exists. | |
| 1658 | var map = try collectSdkEventMap(testing.allocator); | |
| 1659 | defer map.deinit(); | |
| 1660 | ||
| 1661 | var dispatched = try collectSpecDispatchedEvents(testing.allocator); | |
| 1662 | defer dispatched.deinit(); | |
| 1663 | try testing.expect(dispatched.count() >= 17); | |
| 1664 | ||
| 1665 | var it = dispatched.keyIterator(); | |
| 1666 | while (it.next()) |name| { | |
| 1667 | if (!map.contains(name.*)) { | |
| 1668 | std.debug.print( | |
| 1669 | "the spec dispatches `{s}` and craft.d.ts does not type it.\n" ++ | |
| 1670 | " A page listening for it gets a bare Event and a `detail` of any.\n", | |
| 1671 | .{name.*}, | |
| 1672 | ); | |
| 1673 | return error.SdkDoesNotTypeAnEventThatFires; | |
| 1674 | } | |
| 1675 | } | |
| 1676 | } | |