Skip to content

Commit 0266171

Browse files
ochafikclaude
andauthored
Add addEventListener/removeEventListener with DOM-model on* semantics (#573)
* feat(events): DOM-model on* semantics with addEventListener/removeEventListener Fixes #551 (useHostStyles broken: fonts clobber theme/CSS variables) Fixes #225 (user onhostcontextchanged and SDK hooks mutually exclusive) The on* setters now follow the DOM event model: - Replace semantics (like el.onclick) with getters - Coexist with addEventListener/removeEventListener listeners - Both channels fire on dispatch: on* handler then listeners Introduces ProtocolWithEvents base class between Protocol and App/AppBridge. Each notification event gets a slot with two independent channels (singular on* handler + addEventListener listener array). useHostStyles hooks now use addEventListener with proper cleanup, so they compose correctly with user on* handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix typedoc validation warnings - Export AppEventMap, AppBridgeEventMap, ProtocolWithEvents so they appear in generated docs (they're part of public addEventListener signatures) - Move AppBridgeEventMap above the AppBridge class JSDoc — it had been inserted between the doc block and the class, orphaning the AppBridge docs onto the wrong type - Drop @param callback tags from on* getter/setter pairs — typedoc attaches the doc block to the getter signature which has no params - Add MethodSchema to intentionallyNotExported (internal zod helper referenced by the now-documented eventSchemas field) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 14969c5 commit 0266171

7 files changed

Lines changed: 1009 additions & 201 deletions

File tree

src/app-bridge.test.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,4 +1221,180 @@ describe("isToolVisibilityAppOnly", () => {
12211221
expect(isToolVisibilityAppOnly(tool)).toBe(false);
12221222
});
12231223
});
1224+
1225+
describe("addEventListener / removeEventListener", () => {
1226+
let app: App;
1227+
let bridge: AppBridge;
1228+
let appTransport: InMemoryTransport;
1229+
let bridgeTransport: InMemoryTransport;
1230+
1231+
beforeEach(async () => {
1232+
[appTransport, bridgeTransport] = InMemoryTransport.createLinkedPair();
1233+
app = new App(testAppInfo, {}, { autoResize: false });
1234+
bridge = new AppBridge(
1235+
createMockClient() as Client,
1236+
testHostInfo,
1237+
testHostCapabilities,
1238+
);
1239+
await bridge.connect(bridgeTransport);
1240+
});
1241+
1242+
afterEach(async () => {
1243+
await appTransport.close();
1244+
await bridgeTransport.close();
1245+
});
1246+
1247+
it("App.addEventListener fires multiple listeners for the same event", async () => {
1248+
const a: unknown[] = [];
1249+
const b: unknown[] = [];
1250+
app.addEventListener("hostcontextchanged", (p) => a.push(p));
1251+
app.addEventListener("hostcontextchanged", (p) => b.push(p));
1252+
1253+
await app.connect(appTransport);
1254+
bridge.setHostContext({ theme: "dark" });
1255+
await flush();
1256+
1257+
expect(a).toEqual([{ theme: "dark" }]);
1258+
expect(b).toEqual([{ theme: "dark" }]);
1259+
});
1260+
1261+
it("App notification setters replace (DOM onclick model)", async () => {
1262+
const a: unknown[] = [];
1263+
const b: unknown[] = [];
1264+
const first = (p: unknown) => a.push(p);
1265+
app.ontoolinput = first;
1266+
expect(app.ontoolinput).toBe(first);
1267+
app.ontoolinput = (p) => b.push(p);
1268+
1269+
await app.connect(appTransport);
1270+
await bridge.sendToolInput({ arguments: { x: 1 } });
1271+
await flush();
1272+
1273+
// Second assignment replaced the first (like el.onclick)
1274+
expect(a).toEqual([]);
1275+
expect(b).toEqual([{ arguments: { x: 1 } }]);
1276+
});
1277+
1278+
it("App notification setter coexists with addEventListener", async () => {
1279+
const a: unknown[] = [];
1280+
const b: unknown[] = [];
1281+
app.ontoolinput = (p) => a.push(p);
1282+
app.addEventListener("toolinput", (p) => b.push(p));
1283+
1284+
await app.connect(appTransport);
1285+
await bridge.sendToolInput({ arguments: { x: 1 } });
1286+
await flush();
1287+
1288+
// Both the on* handler and addEventListener listener fire
1289+
expect(a).toEqual([{ arguments: { x: 1 } }]);
1290+
expect(b).toEqual([{ arguments: { x: 1 } }]);
1291+
});
1292+
1293+
it("App notification getter returns the on* handler", () => {
1294+
expect(app.ontoolinput).toBeUndefined();
1295+
const handler = () => {};
1296+
app.ontoolinput = handler;
1297+
expect(app.ontoolinput).toBe(handler);
1298+
});
1299+
1300+
it("App notification setter can be cleared with undefined", async () => {
1301+
const a: unknown[] = [];
1302+
app.ontoolinput = (p) => a.push(p);
1303+
expect(app.ontoolinput).toBeDefined();
1304+
app.ontoolinput = undefined;
1305+
1306+
await app.connect(appTransport);
1307+
await bridge.sendToolInput({ arguments: { x: 1 } });
1308+
await flush();
1309+
1310+
expect(a).toEqual([]);
1311+
expect(app.ontoolinput).toBeUndefined();
1312+
});
1313+
1314+
it("App.removeEventListener stops a listener from firing", async () => {
1315+
const a: unknown[] = [];
1316+
const listener = (p: unknown) => a.push(p);
1317+
app.addEventListener("toolinput", listener);
1318+
app.removeEventListener("toolinput", listener);
1319+
1320+
await app.connect(appTransport);
1321+
await bridge.sendToolInput({ arguments: {} });
1322+
await flush();
1323+
1324+
expect(a).toEqual([]);
1325+
});
1326+
1327+
it("App.onEventDispatch merges hostcontext before listeners fire", async () => {
1328+
let seen: unknown;
1329+
app.addEventListener("hostcontextchanged", () => {
1330+
seen = app.getHostContext();
1331+
});
1332+
1333+
await app.connect(appTransport);
1334+
bridge.setHostContext({ theme: "dark" });
1335+
await flush();
1336+
1337+
expect(seen).toEqual({ theme: "dark" });
1338+
});
1339+
1340+
it("AppBridge.addEventListener fires multiple listeners", async () => {
1341+
let a = 0;
1342+
let b = 0;
1343+
bridge.addEventListener("initialized", () => a++);
1344+
bridge.addEventListener("initialized", () => b++);
1345+
1346+
await app.connect(appTransport);
1347+
1348+
expect(a).toBe(1);
1349+
expect(b).toBe(1);
1350+
});
1351+
1352+
it("on* request setters have replace semantics (no throw)", () => {
1353+
app.onteardown = async () => ({});
1354+
expect(() => {
1355+
app.onteardown = async () => ({});
1356+
}).not.toThrow();
1357+
});
1358+
1359+
it("on* request setters have getters", () => {
1360+
expect(app.onteardown).toBeUndefined();
1361+
const handler = async () => ({});
1362+
app.onteardown = handler;
1363+
expect(app.onteardown).toBe(handler);
1364+
});
1365+
1366+
it("direct setRequestHandler throws when called twice", () => {
1367+
const bridge2 = new AppBridge(
1368+
createMockClient() as Client,
1369+
testHostInfo,
1370+
testHostCapabilities,
1371+
);
1372+
bridge2.setRequestHandler(
1373+
// @ts-expect-error — exercising throw path with raw schema
1374+
{ shape: { method: { value: "test/method" } } },
1375+
() => ({}),
1376+
);
1377+
expect(() => {
1378+
bridge2.setRequestHandler(
1379+
// @ts-expect-error — exercising throw path with raw schema
1380+
{ shape: { method: { value: "test/method" } } },
1381+
() => ({}),
1382+
);
1383+
}).toThrow(/already registered/);
1384+
});
1385+
1386+
it("direct setNotificationHandler throws for event-mapped methods", () => {
1387+
const app2 = new App(testAppInfo, {}, { autoResize: false });
1388+
app2.addEventListener("toolinput", () => {});
1389+
expect(() => {
1390+
app2.setNotificationHandler(
1391+
// @ts-expect-error — exercising throw path with raw schema
1392+
{
1393+
shape: { method: { value: "ui/notifications/tool-input" } },
1394+
},
1395+
() => {},
1396+
);
1397+
}).toThrow(/already registered/);
1398+
});
1399+
});
12241400
});

0 commit comments

Comments
 (0)