Skip to content

Commit f121180

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 f121180

File tree

6 files changed

+958
-218
lines changed

6 files changed

+958
-218
lines changed

src/app-bridge.test.ts

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,4 +1221,177 @@ 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+
app.ontoolinput = (p) => a.push(p);
1265+
app.ontoolinput = (p) => b.push(p);
1266+
1267+
await app.connect(appTransport);
1268+
await bridge.sendToolInput({ arguments: { x: 1 } });
1269+
await flush();
1270+
1271+
// Second assignment replaced the first (like el.onclick)
1272+
expect(a).toEqual([]);
1273+
expect(b).toEqual([{ arguments: { x: 1 } }]);
1274+
});
1275+
1276+
it("App notification setter coexists with addEventListener", async () => {
1277+
const a: unknown[] = [];
1278+
const b: unknown[] = [];
1279+
app.ontoolinput = (p) => a.push(p);
1280+
app.addEventListener("toolinput", (p) => b.push(p));
1281+
1282+
await app.connect(appTransport);
1283+
await bridge.sendToolInput({ arguments: { x: 1 } });
1284+
await flush();
1285+
1286+
// Both the on* handler and addEventListener listener fire
1287+
expect(a).toEqual([{ arguments: { x: 1 } }]);
1288+
expect(b).toEqual([{ arguments: { x: 1 } }]);
1289+
});
1290+
1291+
it("App notification getter returns the on* handler", () => {
1292+
expect(app.ontoolinput).toBeUndefined();
1293+
const handler = () => {};
1294+
app.ontoolinput = handler;
1295+
expect(app.ontoolinput).toBe(handler);
1296+
});
1297+
1298+
it("App notification setter can be cleared with undefined", async () => {
1299+
const a: unknown[] = [];
1300+
app.ontoolinput = (p) => a.push(p);
1301+
app.ontoolinput = undefined;
1302+
1303+
await app.connect(appTransport);
1304+
await bridge.sendToolInput({ arguments: { x: 1 } });
1305+
await flush();
1306+
1307+
expect(a).toEqual([]);
1308+
expect(app.ontoolinput).toBeUndefined();
1309+
});
1310+
1311+
it("App.removeEventListener stops a listener from firing", async () => {
1312+
const a: unknown[] = [];
1313+
const listener = (p: unknown) => a.push(p);
1314+
app.addEventListener("toolinput", listener);
1315+
app.removeEventListener("toolinput", listener);
1316+
1317+
await app.connect(appTransport);
1318+
await bridge.sendToolInput({ arguments: {} });
1319+
await flush();
1320+
1321+
expect(a).toEqual([]);
1322+
});
1323+
1324+
it("App.onEventDispatch merges hostcontext before listeners fire", async () => {
1325+
let seen: unknown;
1326+
app.addEventListener("hostcontextchanged", () => {
1327+
seen = app.getHostContext();
1328+
});
1329+
1330+
await app.connect(appTransport);
1331+
bridge.setHostContext({ theme: "dark" });
1332+
await flush();
1333+
1334+
expect(seen).toEqual({ theme: "dark" });
1335+
});
1336+
1337+
it("AppBridge.addEventListener fires multiple listeners", async () => {
1338+
let a = 0;
1339+
let b = 0;
1340+
bridge.addEventListener("initialized", () => a++);
1341+
bridge.addEventListener("initialized", () => b++);
1342+
1343+
await app.connect(appTransport);
1344+
1345+
expect(a).toBe(1);
1346+
expect(b).toBe(1);
1347+
});
1348+
1349+
it("on* request setters have replace semantics (no throw)", () => {
1350+
app.onteardown = async () => ({});
1351+
expect(() => {
1352+
app.onteardown = async () => ({});
1353+
}).not.toThrow();
1354+
});
1355+
1356+
it("on* request setters have getters", () => {
1357+
expect(app.onteardown).toBeUndefined();
1358+
const handler = async () => ({});
1359+
app.onteardown = handler;
1360+
expect(app.onteardown).toBe(handler);
1361+
});
1362+
1363+
it("direct setRequestHandler throws when called twice", () => {
1364+
const bridge2 = new AppBridge(
1365+
createMockClient() as Client,
1366+
testHostInfo,
1367+
testHostCapabilities,
1368+
);
1369+
bridge2.setRequestHandler(
1370+
// @ts-expect-error — exercising throw path with raw schema
1371+
{ shape: { method: { value: "test/method" } } },
1372+
() => ({}),
1373+
);
1374+
expect(() => {
1375+
bridge2.setRequestHandler(
1376+
// @ts-expect-error — exercising throw path with raw schema
1377+
{ shape: { method: { value: "test/method" } } },
1378+
() => ({}),
1379+
);
1380+
}).toThrow(/already registered/);
1381+
});
1382+
1383+
it("direct setNotificationHandler throws for event-mapped methods", () => {
1384+
const app2 = new App(testAppInfo, {}, { autoResize: false });
1385+
app2.addEventListener("toolinput", () => {});
1386+
expect(() => {
1387+
app2.setNotificationHandler(
1388+
// @ts-expect-error — exercising throw path with raw schema
1389+
{
1390+
shape: { method: { value: "ui/notifications/tool-input" } },
1391+
},
1392+
() => {},
1393+
);
1394+
}).toThrow(/already registered/);
1395+
});
1396+
});
12241397
});

0 commit comments

Comments
 (0)