Automattic / studio

Studio by WordPress.com, a free desktop app that helps developers streamline their local WordPress development workflow.
https://developer.wordpress.com/studio/
GNU General Public License v2.0
193 stars 18 forks source link

Improve performance of Assistant tab #550

Closed fluiddot closed 1 month ago

fluiddot commented 1 month ago

Related to https://github.com/Automattic/studio/pull/549 (this issue was spotted while testing that PR) and https://github.com/Automattic/dotcom-forge/issues/9207.

Proposed Changes

Testing Instructions


- Run the app with the command `npm start`.
- Open the DevTools.
- Log in to WPCOM with an A8C account.
- Navigate to the Assitant tab.
- Focus on the input field.
- Type some letters.
- Observe that no extra logs are recorded when typing.
- Type quickly several words.
- Observe that the app slows down.
- Submit a request.
- Check the network request made to `wpcom/v2/studio-app/ai-assistant/chat`.
- Observe in the request payload that the `messages` field contains the full prompt.

## Pre-merge Checklist

<!--
Complete applicable items on this checklist **before** merging into trunk. Inapplicable items can be left unchecked.

Both the PR author and reviewer are responsible for ensuring the checklist is completed.
-->

- [x] Have you checked for TypeScript, React or other console errors?
fluiddot commented 1 month ago

I tested this PR and it works, but only for chats that already have messages. Chat, which doesn't have a conversation started, still re-renders.

Should we have a custom comparison function for AuthenticatedView?

Ah, interesting. I'll take a look to figure out how to prevent re-renders in that case. From my testing, this case seems to have less impact on performance as the content being re-rendered is lighter than when having several messages.

While testing this PR, I noticed another issue - entered text is preserved across the sites. I added Automattic/dotcom-forge#9208 for that.

Good catch and thanks for creating the GitHub issue 🙇 !

fluiddot commented 1 month ago

Should we have a custom comparison function for AuthenticatedView?

@wojtekn I've addressed this issue in https://github.com/Automattic/studio/pull/550/commits/8c14f0decaa4dd3f675b1863b62a5dc263fd378c.

fluiddot commented 1 month ago

[...] The only thing I don't think I fully understand is why the AuthenticatedView is rendering twice as soon as I access the Assistant tab with no messages 🤔 Perhaps this is a different issue though:

Screenshot 2024-09-24 at 10 02 42 AM

I checked in React Profile and AuthenticatedView component is only rendered once when navigating to the Assistant tab. I'd like to note that checking re-renders with console logs might not be accurate in some cases.

katinthehatsite commented 1 month ago

I'd like to note that checking re-renders with console logs might not be accurate in some cases.

Interesting, thanks for checking @fluiddot - that sounds like the case I was running into 👍