Skip to content

Commit 9aee887

Browse files
ochafikclaude
andcommitted
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>
1 parent 14969c5 commit 9aee887

6 files changed

Lines changed: 922 additions & 184 deletions

File tree

src/app-bridge.test.ts

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,4 +1221,179 @@ 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+
app.ontoolinput = undefined;
1304+
1305+
await app.connect(appTransport);
1306+
await bridge.sendToolInput({ arguments: { x: 1 } });
1307+
await flush();
1308+
1309+
expect(a).toEqual([]);
1310+
expect(app.ontoolinput).toBeUndefined();
1311+
});
1312+
1313+
it("App.removeEventListener stops a listener from firing", async () => {
1314+
const a: unknown[] = [];
1315+
const listener = (p: unknown) => a.push(p);
1316+
app.addEventListener("toolinput", listener);
1317+
app.removeEventListener("toolinput", listener);
1318+
1319+
await app.connect(appTransport);
1320+
await bridge.sendToolInput({ arguments: {} });
1321+
await flush();
1322+
1323+
expect(a).toEqual([]);
1324+
});
1325+
1326+
it("App.onEventDispatch merges hostcontext before listeners fire", async () => {
1327+
let seen: unknown;
1328+
app.addEventListener("hostcontextchanged", () => {
1329+
seen = app.getHostContext();
1330+
});
1331+
1332+
await app.connect(appTransport);
1333+
bridge.setHostContext({ theme: "dark" });
1334+
await flush();
1335+
1336+
expect(seen).toEqual({ theme: "dark" });
1337+
});
1338+
1339+
it("AppBridge.addEventListener fires multiple listeners", async () => {
1340+
let a = 0;
1341+
let b = 0;
1342+
bridge.addEventListener("initialized", () => a++);
1343+
bridge.addEventListener("initialized", () => b++);
1344+
1345+
await app.connect(appTransport);
1346+
1347+
expect(a).toBe(1);
1348+
expect(b).toBe(1);
1349+
});
1350+
1351+
it("on* request setters have replace semantics (no throw)", () => {
1352+
app.onteardown = async () => ({});
1353+
expect(() => {
1354+
app.onteardown = async () => ({});
1355+
}).not.toThrow();
1356+
});
1357+
1358+
it("on* request setters have getters", () => {
1359+
expect(app.onteardown).toBeUndefined();
1360+
const handler = async () => ({});
1361+
app.onteardown = handler;
1362+
expect(app.onteardown).toBe(handler);
1363+
});
1364+
1365+
it("direct setRequestHandler throws when called twice", () => {
1366+
const bridge2 = new AppBridge(
1367+
createMockClient() as Client,
1368+
testHostInfo,
1369+
testHostCapabilities,
1370+
);
1371+
bridge2.setRequestHandler(
1372+
// @ts-expect-error — exercising throw path with raw schema
1373+
{ shape: { method: { value: "test/method" } } },
1374+
() => ({}),
1375+
);
1376+
expect(() => {
1377+
bridge2.setRequestHandler(
1378+
// @ts-expect-error — exercising throw path with raw schema
1379+
{ shape: { method: { value: "test/method" } } },
1380+
() => ({}),
1381+
);
1382+
}).toThrow(/already registered/);
1383+
});
1384+
1385+
it("direct setNotificationHandler throws for event-mapped methods", () => {
1386+
const app2 = new App(testAppInfo, {}, { autoResize: false });
1387+
app2.addEventListener("toolinput", () => {});
1388+
expect(() => {
1389+
app2.setNotificationHandler(
1390+
// @ts-expect-error — exercising throw path with raw schema
1391+
{
1392+
shape: { method: { value: "ui/notifications/tool-input" } },
1393+
},
1394+
() => {},
1395+
);
1396+
}).toThrow(/already registered/);
1397+
});
1398+
});
12241399
});

0 commit comments

Comments
 (0)