feat(client): widen the client tool return type to what the SDK coerces - #986
chinmayv095 wants to merge 1 commit into
Conversation
BaseConversation serialises an object result with JSON.stringify and sends everything else through String, but ClientToolsConfig declared the return type as string | number | void, so a handler returning an object or a boolean was a type error for behaviour the SDK supports on purpose. Add an exported ClientToolResult covering what the coercion handles, and have @elevenlabs/react re-export it rather than keep its own narrower copy, so the hook API and the client agree on what a tool may return.
PR SummaryLow Risk Overview
Adds tests that pin the outgoing Reviewed by Cursor Bugbot for commit 392ef65. Bugbot is set up for automated code reviews on this repo. Configure here. |
Fixes #972.
BaseConversationhas serialised object results from client tools since #41, and that PR says so on purpose: "The API expects the result of a client tool to always be a string, this enforces that on the SDK level." The declared type never followed.So returning an object, an array or a boolean is supported behaviour that TypeScript rejects, and a caller has to
JSON.stringifyon their own side to satisfy a type whose only consumer stringifies it again.The two copies of the type had drifted
@elevenlabs/reactdeclares its own:while
ConversationClientTools.tsxstores handlers asClientToolsConfig["clientTools"][string], which is the client's type. Both were narrow in the same way, so the duplication was invisible. Widening one of them makes it visible at once: a handler that satisfiesclientToolsstops satisfyingClientTool, andClientToolis whatuseConversationClientTooltakes. Measured by reverting only the react file on this branch:So the fix belongs in both, from one definition.
@elevenlabs/clientnow exportsClientToolResultand@elevenlabs/reactre-exports it instead of keeping its own.What the type says now
objectcovers arrays, whichJSON.stringifyalready handles.booleanis in there becauseString(false)is"false"andfalseis not nullish, so it never reaches the?? "Client tool execution successful."default. That is easy to get wrong by reasoning about??alone, so it has its own test.The issue offered
Promise<unknown> | unknownas one option and this narrower form as another. I took the narrower one:unknownwould type-check the same handlers but stop describing the coercion, and describing it is the only job this annotation has.Verification
Nothing changes at runtime, so the type check is the discriminating evidence here, not the test run. Both are included.
@elevenlabs/clientand 3 in@elevenlabs/react. They had to be measured separately because the client build fails first. With the change,pnpm -w check-typesis green across all 16 tasks.BaseConversation.test.tscovering an object, an array, a boolean and a handler that returns nothing, asserting the exactclient_tool_resultpayload sent. These pass with and without the change, deliberately: they pin the behaviour the type now claims, so a later narrowing of the coercion fails too. Two compile-time assertions carry the part that does discriminate.pnpm -w lintgreen across all 29 tasks. 262 client tests and 142 react tests pass.convai-widget-core'sDismissButton.test.ts. Confirmed pre-existing by stashing this branch's changes and getting the identical failure on a clean tree.Changeset included as a
minorfor both packages, since this adds an export and widens two published types.