fix: make threadId optional in sendThreadMessage #292
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
The changes improve backwards compatibility by making threadId optional. However, there is a potential inconsistency in the defaulting logic between the default parameter and the destructuring assignment that could lead to unexpected behavior.
Summary of changes
Summary of Changes
- Made
threadIdoptional in theTamboThreadContextPropsinterface and in thesendThreadMessagefunction. - Updated the default behavior so that if
threadIdis undefined in the options object, it now defaults tocurrentThread.idduring destructuring. - Adjusted the dependency array of the
useCallbackhook to usecurrentThread.idinstead of the wholecurrentThreadobject.
| streamResponse?: boolean; | ||
| contextKey?: string; | ||
| } = { threadId: PLACEHOLDER_THREAD.id }, | ||
| ): Promise<TamboThreadMessage> => { | ||
| const { threadId, streamResponse } = options; | ||
| const { threadId = currentThread.id, streamResponse } = options; |
There was a problem hiding this comment.
The default assignment for threadId is handled in two places: the default parameter ({ threadId: PLACEHOLDER_THREAD.id }) and the destructuring default (threadId = currentThread.id). This may lead to inconsistent behavior when a caller explicitly passes an empty object versus when the parameter is omitted. Ensure that this discrepancy is intentional for backwards compatibility and consider unifying the defaults to avoid potential confusion.
Suggestion
Consider using a unified default value for threadId across both the default parameter and the destructuring. For example, change the default parameter to use currentThread.id:
} = { threadId: currentTh
AB49
read.id },Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
fixes some backwards compatibility with 0.19