-
Notifications
You must be signed in to change notification settings - Fork 11k
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
webui: Stop rerender on textarea input and end the devastating lag #12299
base: master
Are you sure you want to change the base?
webui: Stop rerender on textarea input and end the devastating lag #12299
Conversation
@@ -99,12 +99,15 @@ export default function ChatScreen() { | |||
canvasData, | |||
replaceMessageAndGenerate, | |||
} = useAppContext(); | |||
const [inputMsg, setInputMsg] = useState(prefilledMsg.content()); |
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.
If I understand correctly, prefilledMsg.content()
is currently discarded
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.
Could you rephrase what you mean? I put prefilledMsg.content()
below under defaultValue
No it does not rerender the whole screen. In fact, the rerender of the textarea alone is already slow, and this is a known problem in reactjs: https://www.google.com/search?client=firefox-b-d&q=reactjs+textarea+slow I'm a bit surprise that there is no solution (i.e. pre-made components) to solve this problem. I think I'll propose something more elegant (maybe
I think it's mostly due to the logic where we extract |
Sorry for the hyperbole, I must have been overly emotional this morning haha.
I think the whole point of my fix and why it works is that it no longer rerenders the entire ChatScreen component on input? (Yes I only meant ChatScreen and not the whole page) When I search for slow textarea input in React, it all has to do with textareas in React that are doing what is currently on master: rerendering the component if something changes by using setXXX (from useState) to update the state on input change. Anyway, thank you very very much for putting some attention to this problem since I encounter it a lot :) |
haha no worries, I'm kinda on the same boat recently Indeed, your approach in this PR is good, I think if anyone ever create "pre-made component" that I mentioned in the last message, they would do the same. I think it will be cleaner to wrap your logic into a new component called |
Ok I still haven't been able to come up with a better way to abstract this out, will revisit this tomorrow then. |
@@ -259,8 +267,7 @@ export default function ChatScreen() { | |||
className="textarea textarea-bordered w-full" | |||
placeholder="Type a message (Shift+Enter to add a new line)" | |||
ref={inputRef} | |||
value={inputMsg} | |||
onChange={(e) => setInputMsg(e.target.value)} | |||
defaultValue={prefilledMsg.content()} |
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.
There could be a race condition between prefilledMsg.content()
and prefilledMsg.clear()
above
Fixes the devastating lag when typing into the textarea by making it uncontrolled and stopping us from rerendering the WHOLE chat screen whenever you type a character into the input textarea.
Closes #11813 #12026
I did NOT test how this affects the useVSCodeContext integration. Also with this PR the send button does not disable, but that's a lower priority. If this functionality is absolutely necessary then you need to extract something (the button and maybe textarea) into another component.
It seems like there is still lots of lag in general with rendering (when the assistant response is being printed for example) but at least this cuts it down on the textarea input lag :/