Skip to content
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

AI Chat: refactor redux file #63775

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

sanchitmalhotra126
Copy link
Contributor

@sanchitmalhotra126 sanchitmalhotra126 commented Feb 6, 2025

This is an attempt to break up our nearly 900-line redux file into smaller, targeted files, grouped by type (selectors, thunks). We still export a single index.ts file in the aichat/redux directory so other components can still just import from aichat/redux directly.

Details:

  • Thunks are moved into a redux/thunks folder, with a separate file per thunk. Some of the thunks are quite large, so this helps organize things (even if others are pretty small)
    • Moved any shared thunk helper functions into a redux/thunks/helpers subdirectory
  • Selectors are now in a redux/selectors/index.ts file. I opted to keep these in one file since they're individually pretty small.
  • The remainder of aichatRedux is just the state schema and slice definition, so I renamed it slice.ts. It's still not that small, but is focused on just defining the state and actions. I debated pulling out AichatState into a separate file too but kept it there for now. Happy to reconsider though.
    • Previously, we only exported a subset of redux actions since we only wanted those to be used by component code (the rest were only used by thunks and other internal redux code). Now this selective export happens in the redux/index.ts file.

Like #63710 this is mostly copy+paste only, but there were a few minor changes needed to get everything working correctly:

  • Imports are now from aichat/redux instead of aichat/redux/aichatRedux
  • I removed the waitingForResponse: boolean state value, and instead just compute this based on the presence of a chatMessagePending. This was primarily so I could get rid of the extraReducers in the slice and avoid a circular dependency between the thunk file and the slice file. We set waitingForResponse to true only when we're in the process of sending a chat message, which is also only when chatMessagePending is defined, so I felt this made sense.

Testing story

Tested locally. No functional/behavioral changes.

Follow-up work

I'd like to clean up the utils.ts file too, but I'll leave this for a separate change that tackles all the various util files together - there's a bunch of util files spread out across these three files that all kind of get used in different places.

@sanchitmalhotra126 sanchitmalhotra126 requested review from bencodeorg and a team February 6, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant