-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat (svelte): refactor to use ChatStore
#6329
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
Conversation
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
| import { Chat } from '@ai-sdk/svelte'; | ||
| import { defaultChatStore } from 'ai'; | ||
| const chat = new Chat(() => ({ |
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.
@elliott-with-the-longest-name-on-github the chat class tests pass but i'm seeing funky behavior w/ re-rendering message part changes when running the example!
if you have time to look or if you have time to sync again tomorrow lmk!
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.
seems like under the hood, sveltekit's optimizations for only re-rendering when state changes doesn't account for the deeply nested message parts changes
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.
(for context: chat store's getMessages just returns chat.messages!)
| if (!this.hasChat(id)) { | ||
| throw new Error(`chat '${id}' not found`); | ||
| this.addChat(id, []); | ||
| } |
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.
@lgrammel we decided to add this logic to the chat store itself! this resolves a concern w/ the svelte approach where upon switching chats after Chat class is initialized, new chat id's did not exist in the store
feat: Better multi-framework support for fine-grained / signal-based reactivity systems
|
|
||
| const state = createStreamingUIMessageState({ | ||
| lastMessage, | ||
| lastMessage: structuredClone(lastMessage), |
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.
Moved structuredClone of last message out of createStreamingUIMessageState (explanation in corresponding file)
| }) { | ||
| const state = createStreamingUIMessageState({ | ||
| lastMessage, | ||
| lastMessage: lastMessage ? structuredClone(lastMessage) : undefined, |
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.
Same comment as above
| InferUIDataParts<UI_DATA_PART_SCHEMAS> | ||
| > = isContinuation | ||
| ? structuredClone(lastMessage) | ||
| ? lastMessage |
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.
Removed because this was breaking the Svelte impl.
Quote from @elliott-with-the-longest-name-on-github:
"The problem is that structuredClone doesn't work on Svelte state (because it's a Proxy, which isn't cloneable)"
| createAIContext(); | ||
| const chat1 = new Chat({ chatId }); | ||
| const chat2 = new Chat({ chatId }); | ||
| createAIContext(defaultChatStore({ api: '/api/chat' })); |
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.
@elliott-with-the-longest-name-on-github I believe we won't need this and the corresponding store context provider anymore, but want to confirm with you!
| }; | ||
|
|
||
| export class ChatStore< | ||
| export interface ChatStateManagerFactory< |
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.
lol
| }, | ||
| "peerDependencies": { | ||
| "svelte": "^5.0.0", | ||
| "svelte": "^5.31.0", |
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.
why is this needed?
## Background Use the new chat store concept in Svelte to increase code reuse and standardize. ## Summary - refactor chat store to allow for react and svelte optimizations - refactor initialization pattern to fix react issue with overrides on render ## Verification - [x] react with tool calls - [x] svelte with tool calls ## Future Work - vue - refine svelte and react implementations ## Related Issues Continues #6329 --------- Co-authored-by: Grace Yun <graceyunn@gmail.com> Co-authored-by: Grace Yun <74513600+iteratetograceness@users.noreply.github.com> Co-authored-by: S. Elliott Johnson <sejohnson@torchcloudconsulting.com>
|
Closing, changes shipped in #6487 |
Background
Refactor
@ai-sdk/sveltepackage to use newChatStoreandChatTransportSummary
This PR:
More detailed doc, written by @elliott-with-the-longest-name-on-github found here: https://www.notion.so/vercel/An-Essay-on-ai-sdk-svelte-3-and-ai-5-1fbe06b059c480e1bda6dbf3a613a0a9
As a result of 2, we refactored the React implementation as well to integrate the State Manager, where methods strictly adhere to immutability.
This also sets up the Vue refactor to be quite simple!
Verification
Tasks
pnpm changesetin the project root)pnpm prettier-fixin the project root)Future Work
Related Issues