Migrate to @tambo-ai/react 1.0.0-rc.4#24
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
5a12d90 to
18357a2
Compare
There was a problem hiding this comment.
The migration is broadly consistent with the V1 SDK model (hooks, streaming state, content blocks), but there are several correctness/UX regressions: thread renaming UI is now non-functional, tool call expansion state is shared across all tool calls, and cancellation handling is applied too broadly (can hide previously generated components). There are also risks around React list keys (server?.url), and a couple of maintainability concerns (as any casts, nested ternary stage mapping). Address the lastRunCancelled gating and rename UI before merging.
Additional notes (5)
- Maintainability |
src/components/tambo/message.tsx:429-525
Logic correctness: tool call expansion state is shared across all tool calls
ToolcallInfo now renders multiple tool_use blocks, but uses a single isExpanded state for all of them. Clicking one tool call expands/collapses every tool call in that message, which is usually not the intended UX and makes it harder to inspect a specific call.
Also, hasToolError is hardcoded to false, so tool failures will never be reflected in the status icon/label even if results contain error info.
- Performance |
src/components/tambo/message.tsx:437-453
Performance/behavior risk: O(N*M) scan across all messages for tool results
associatedToolResults scans every message and every content block on each render to find matching tool_result blocks. In longer threads this can become noticeably expensive, especially during streaming where renders are frequent.
At minimum, consider indexing tool results once per messages change (e.g. build a map from toolUseId → result) and then doing O(1) lookups per tool block. This also simplifies and avoids storing arrays of results when toolUseId is unique.
- Maintainability |
src/lib/tambo.ts:83-96
Unsafe typing: component: Graph as any / SelectForm as any
This introduces any into a core integration layer (src/lib/tambo.ts), which is a long-lived sharp edge: it can mask SDK contract changes and make future migrations harder.
Given the project explicitly migrated to V1 SDK, it’s better to adapt the component types at the boundary with a narrow, documented cast (or adjust the component definitions) rather than broad any.
- Maintainability |
src/components/tambo/thread-history.tsx:442-442
Behavioral regression: thread renaming UI now does nothing
ThreadHistoryList still renders a rename flow and collects newName, but handleNameSubmit now just calls setEditingThread(null) without persisting the name. This leaves a broken UI affordance (users can "rename" but it won't stick), which is worse than removing the option.
If V1 removed thread renaming, hide/disable the rename menu item and editing UI. If renaming is still possible via a new API, wire it up.
- Readability |
src/components/tambo/thread-content.tsx:81-88
Readability/maintainability: nested ternary mapping streaming status → legacy generation stages
generationStage is being synthesized from streamingState.status using nested ternaries. This is hard to read and brittle if more statuses are added.
If you still need a legacy-ish stage, prefer a small mapping function/object (and ideally centralize it so other components don’t each reinvent the mapping).
Summary of changes
What changed
SDK migration: @tambo-ai/react V1
- Upgraded
@tambo-ai/reactfrom^0.65.2to^1.0.0-rc.4(and updated the lockfile dependency graph accordingly). - Migrated legacy thread APIs:
useTamboThread→useTambo()- Thread
contextKeyusage removed fromMessageThreadFull,MessageInput, andThreadHistory. - Updated streaming/generation UI to use V1 streaming state (e.g.
isStreaming,isWaiting,streamingState.status).
Thread UX & rendering model updates
- Tool calls migrated from
message.toolCallRequest/role: "tool"to V1 content blocks (tool_use/tool_result) with helpers likegetToolUseBlocks(). - Rendered components now pulled from
componentcontent blocks instead ofmessage.renderedComponent, with a simplifiedextractComponentInfo(). - Autoscroll logic updated to depend on
messagesandisStreamingrather than legacy generation stage.
New anonymous user identity
- Added
src/lib/use-anonymous-user-key.tsto persist a stable anonymoususerKeyinlocalStorageand wired it into<TamboProvider userKey={...} />.
Interactables & MCP wiring
- Updated interactable HOCs from
withInteractable→withTamboInteractable. - Adjusted mention/context attachment payload shape in the text editor.
Docs
- README example updated to remove
contextKeyfrom<MessageThreadFull />usage.
| {filteredResources.map((resourceEntry) => ( | ||
| <DropdownMenu.Item | ||
| key={`${resourceEntry.server.url}-${resourceEntry.resource.uri}`} | ||
| key={`${resourceEntry.server?.url}-${resourceEntry.resource.uri}`} | ||
| className="relative flex cursor-pointer select-none items-start flex-col rounded-sm px-2 py-2 text-sm outline-none hover:bg-accent hover:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50 focus:bg-accent focus:text-accent-foreground" |
There was a problem hiding this comment.
Bug: key may become unstable/duplicate when server is undefined
Changing the key from ${resourceEntry.server.url}-... to ${resourceEntry.server?.url}-... can collapse multiple items into the same key prefix ("undefined-...") when server is missing, which can cause React list reconciliation bugs (incorrect item reuse, wrong hover/selection behavior).
If server can be absent, the key should be derived from a stable unique identifier that is always present (e.g. resource.uri alone, or a server id/name fallback).
Suggestion
Use a key that is always stable and unique even when server is missing. For example:
- Prefer
resourceEntry.resource.urialone if it's globally unique. - Or add a deterministic fallback:
resourceEntry.server?.url ?? resourceEntry.server?.name ?? 'unknown-server'.
Example:
key={`${resourceEntry.resource.uri}`}
// or
key={`${resourceEntry.server?.url ?? resourceEntry.server?.name ?? "unknown"}-${resourceEntry.resource.uri}`}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| const isCancelled = thread?.thread?.lastRunCancelled ?? false; | ||
|
|
||
| if ( | ||
| !message.renderedComponent || | ||
| !renderedComponent || | ||
| role !== "assistant" || | ||
| message.isCancelled | ||
| isCancelled | ||
| ) { | ||
| return null; |
There was a problem hiding this comment.
Bug: cancellation flags are applied too broadly
MessageRenderedComponentArea now hides rendered components when thread?.thread?.lastRunCancelled is true. That flag appears to be thread-level and may indicate the last run was cancelled, but not necessarily that this particular message/component is invalid.
As written, cancelling a run can suppress components for all assistant messages (including previously completed ones), which is likely incorrect and will confuse users (components "disappear" after a cancellation).
You already implemented more precise logic in MessageContent by checking the last assistant message id; this should use a similarly precise guard.
Suggestion
Apply cancellation UI/behavior only to the message that corresponds to the cancelled run (typically the last assistant message in the thread), instead of using a thread-level boolean to hide all components.
You can reuse the same isLastAssistantMessage computation and gate on that:
const isLastAssistantMessage = /* same logic as MessageContent */;
const wasCancelled = isLastAssistantMessage && (thread?.thread.lastRunCancelled ?? false);
if (!renderedComponent || role !== "assistant" || wasCancelled) return null;Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| function getOrCreateUserKey(): string { | ||
| if (typeof window === "undefined") { | ||
| return ""; | ||
| } | ||
| const existing = localStorage.getItem(STORAGE_KEY); | ||
| if (existing) { | ||
| return existing; | ||
| } | ||
| const newKey = crypto.randomUUID(); | ||
| localStorage.setItem(STORAGE_KEY, newKey); | ||
| return newKey; | ||
| } | ||
|
|
||
| export function useAnonymousUserKey(): string { | ||
| const [userKey] = useState(getOrCreateUserKey); | ||
| return userKey; |
There was a problem hiding this comment.
Potential runtime issue: crypto.randomUUID() availability / non-secure contexts
crypto.randomUUID() is not available in all environments (older browsers) and can behave differently depending on secure context. While this is a client hook, if this template is used broadly, a fallback would make it more robust.
Also, returning an empty string on the server means userKey is initially "" during SSR/hydration; if TamboProvider uses userKey immediately, this can cause an initial unauthenticated/anonymous identity mismatch until the client state initializes.
Suggestion
Consider:
- A fallback for UUID generation (e.g.
crypto.getRandomValues+ manual UUID v4) whenrandomUUIDis missing. - Returning
undefinedinstead of""server-side and only passinguserKeytoTamboProvideronce available (or allow the provider to generate its own).
Example tweak:
if (typeof window === "undefined") return undefined as unknown as string;
const newKey = crypto.randomUUID?.() ?? fallbackUuid();(or make the hook return string | undefined and conditionally set the prop).
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with a safe UUID fallback + optional userKey wiring.
…ub optional peer dependencies - Added ESLint configuration to ignore during builds. - Updated Webpack configuration to stub optional peer dependencies from @standard-community/standard-json.
Summary
@tambo-ai/reactfrom legacy API to1.0.0-rc.4(V1 SDK)useTamboThread→useTambo(), content block model, streaming statecontextKey,SamplingSubThread, and migration commentsuse-anonymous-user-key.tsutility for anonymous user key managementTest plan
npx tsc --noEmitpasses with 0 errorsnpm run devstarts without errors