-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(ai-sdk/react): Fix subscribeToMessages callback dependency in useChat #10908
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: main
Are you sure you want to change the base?
fix(ai-sdk/react): Fix subscribeToMessages callback dependency in useChat #10908
Conversation
| expect(screen.queryByTestId('message-0')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should handle streaming correctly when the id changes', async () => { |
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.
This test is largely copied from the test added in #7387:
ai/packages/react/src/use-chat.ui.test.tsx
Lines 2352 to 2415 in 68d8803
| it('should handle streaming correctly when id changes from undefined to defined', async () => { | |
| const controller = new TestResponseController(); | |
| server.urls['/api/chat'].response = { | |
| type: 'controlled-stream', | |
| controller, | |
| }; | |
| // First, change the ID from undefined to 'chat-123' | |
| await userEvent.click(screen.getByTestId('change-id')); | |
| // Then send a message | |
| await userEvent.click(screen.getByTestId('send-message')); | |
| await waitFor(() => { | |
| expect(screen.getByTestId('status')).toHaveTextContent('submitted'); | |
| }); | |
| controller.write(formatChunk({ type: 'text-start', id: '0' })); | |
| controller.write( | |
| formatChunk({ type: 'text-delta', id: '0', delta: 'Hello' }), | |
| ); | |
| // Verify streaming is working - text should appear immediately | |
| await waitFor(() => { | |
| expect( | |
| JSON.parse(screen.getByTestId('messages').textContent ?? ''), | |
| ).toContainEqual( | |
| expect.objectContaining({ | |
| role: 'assistant', | |
| parts: expect.arrayContaining([ | |
| expect.objectContaining({ | |
| type: 'text', | |
| text: 'Hello', | |
| }), | |
| ]), | |
| }), | |
| ); | |
| }); | |
| controller.write(formatChunk({ type: 'text-delta', id: '0', delta: ',' })); | |
| controller.write( | |
| formatChunk({ type: 'text-delta', id: '0', delta: ' world' }), | |
| ); | |
| controller.write(formatChunk({ type: 'text-delta', id: '0', delta: '.' })); | |
| controller.write(formatChunk({ type: 'text-end', id: '0' })); | |
| controller.close(); | |
| await waitFor(() => { | |
| expect( | |
| JSON.parse(screen.getByTestId('messages').textContent ?? ''), | |
| ).toContainEqual( | |
| expect.objectContaining({ | |
| role: 'assistant', | |
| parts: expect.arrayContaining([ | |
| expect.objectContaining({ | |
| type: 'text', | |
| text: 'Hello, world.', | |
| state: 'done', | |
| }), | |
| ]), | |
| }), | |
| ); | |
| }); | |
| }); |
| }; | ||
|
|
||
| // First, change the ID | ||
| await userEvent.click(screen.getByTestId('do-change-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.
is this changing the id on the chat instance in the way you describe in the bug report? ie without your fix would this test fail?
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.
Yes, you can see here it resets the chat instance to one with a new ID:
ai/packages/react/src/use-chat.ui.test.tsx
Lines 2252 to 2262 in fa8e472
| <button | |
| data-testid="do-change-chat" | |
| onClick={() => { | |
| setChat( | |
| new Chat({ | |
| id: 'second-id', | |
| generateId: mockId(), | |
| }), | |
| ); | |
| }} | |
| /> |

Background
Follow-up to:
That PR fixed stale subscriptions when the chat ID changes. However, it was incomplete since it only handled the case where the new ID was passed in the options. It's also possible for you to pass in a
chatinstance directly which could have a new ID.Summary
Tweak the logic there to use the
chatRef.current.idin the dependency array, so no matter how the ID changes we'll refresh the subscription callbacks.Manual Verification
Didn't do manually testing since the unit tests for this specific case (plus the one I added) seemed sufficient.
Checklist
pnpm changesetin the project root)