-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat (core, react): refactor useChat
to use MessageStore
#5770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v5
Are you sure you want to change the base?
feat (core, react): refactor useChat
to use MessageStore
#5770
Conversation
@@ -0,0 +1,237 @@ | |||
import { LanguageModelV2FinishReason } from '@ai-sdk/provider'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: TESTS
packages/react/src/use-chat.ts
Outdated
@@ -8,17 +8,24 @@ import type { | |||
UseChatOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review packages/react/src/use-chat.ui.test.tsx
to see if there are additional test cases to add
useSyncExternalStore
useChat
to use MessageStore
* Internal class for managing UIMessages | ||
*/ | ||
export class MessagesStore { | ||
chatId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would it take to make messages store multi-chat ready? i.e. being able to switch between chats with different ids? (we have many bugs in that area)
this might mean storing additional information such as state
here (and potentially renaming to ChatStore
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would love to chat about this further next week!
this.subscribers = new Set(); | ||
this.notify = throttleMs | ||
? throttle(() => this.emit(), throttleMs) | ||
: () => this.emit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are emit
, subscribers
, and notify
related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subscribers holds all the callbacks we call when we want to notify of changes! notify is a wrapper around the internal emit method that is throttled if throttleMs is passed in!
Direction is great - might be good to explore how this could look like as a more complete chat store, and also check if this will work with Svelte and Vue. |
packages/ai/core/util/chat-store.ts
Outdated
import { throttle } from './throttle'; | ||
|
||
interface ChatStoreSubscriber { | ||
onChatMessagesChanged(): void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be async or not?
Open questions:
Things to note:
Background
Summary
Tasks
pnpm prettier-fix
to fix any formatting issuesFuture Work