Switch from Redux to React Query + Context

[kdmccormick]

We know want to do this before Verawood, or probably even earlier so that Verawood Libraries work can build on top of it.

We agree that it'd be best to do this before the frontend-base conversion in order to de-risk that conversion.

Parts of the redux state to migrate

• courseDetail - refactor: Migration of course details to React query #2724
• customPages
• discussions
• assets
• pagesAndResources
• scheduleAndDetails
• advancedSettings
• studioHome (needs refactor)

  • One thing I'd like to get rid of entirely is the weird "Studio Home" state which has become a kitchen sink of data (course list, library list, user permissions, feature flags, branding/theming). I think the idea was to improve loading times by loading all the data needed to render the studio home page from a single API, but it's just messy now. And today a few small parallel requests that are definitely needed are better than one big request that ends up loading unused data. So that should be broken up into smaller things (course list query, library list query, users permissions query, etc.) and more specific contexts.

• models
  • courseApps
  • liveApps
  • liveAppConfigs
  • XpertSettings
  • courseDetails - refactor: Migration of course details to React query #2724
  • sequences
  • assets
  • videos
  • discussionTopics
• live
• courseTeam
• courseUpdates
• processingNotification
• helpUrls - refactor: Migrate Help Urls to React Query #2714
• courseExport
• courseOptimizer
• generic
• courseImport
• videos
• courseOutline - started in PR #2830, lots still to do
• courseUnit
• courseChecklist
• accessibilityPage - Switch from Redux to React Query + Context #2540
• certificates
• groupConfigurations
• textbooks
• editor state is in a separate ticket: Tracking Issue: Refactors for Visual Editors #2088

Other tasks:

• Remove redux and related packages from this repo's dependencies.
• Consider installing @tanstack/react-query-devtools ?

[ormsbee]

About a year ago, we had a new look-and-feel that we were discussing for the course listing pages:

• https://www.figma.com/design/RDsCFF54oD8cbSLRoLKOCw/Studio-Home?node-id=163-13934&p=f&t=8XZ3qv11VUdZKpUc-0
• https://openedx.atlassian.net/wiki/spaces/COMM/pages/4569726979/2024-10-31+-+UX+UI+Working+Group+Meeting

One possibility is that we implement the new course listing look and feel in React Query and switch over to it, rather than converting the existing course listing.

(Links courtesy of @sdaitzman.)

[brian-smith-tcril]

In order to make reviewing this manageable this work should be split up as much as possible.
Context: as of writing, there are 354 files in this repo that include the string "redux", and 53 that include "react-query"

There is also client-side state being managed by redux that doesn't make sense to try to use react-query for, and should be updated to use Context (see #2091).

[bradenmacdonald]

The work for refactoring the editors code is mostly broken down in #2088 although the tasks can be broken down further if needed.

For the rest of the work, the redux state itself can be used as a guide on how to split up the work into about 25 parts:

frontend-app-authoring/src/store.ts

Lines 40 to 65 in 77215ee

	courseDetail: InferState<typeof courseDetailReducer>;
	customPages: Record<string, any>;
	discussions: Record<string, any>;
	assets: Record<string, any>;
	pagesAndResources: Record<string, any>;
	scheduleAndDetails: Record<string, any>;
	advancedSettings: Record<string, any>;
	studioHome: InferState<typeof studioHomeReducer>;
	models: Record<string, any>;
	live: Record<string, any>;
	courseTeam: Record<string, any>;
	courseUpdates: Record<string, any>;
	processingNotification: Record<string, any>;
	helpUrls: Record<string, any>;
	courseExport: Record<string, any>;
	courseOptimizer: Record<string, any>;
	generic: Record<string, any>;
	courseImport: Record<string, any>;
	videos: Record<string, any>;
	courseOutline: Record<string, any>;
	courseUnit: Record<string, any>;
	courseChecklist: Record<string, any>;
	accessibilityPage: Record<string, any>;
	certificates: Record<string, any>;
	groupConfigurations: InferState<typeof groupConfigurationsReducer>;
	textbooks: Record<string, any>;

Some of these represent a single "Page" in Studio, and migrating each such page to use React Query + React Context should hopefully be straightforward.

One thing I'd like to get rid of entirely is the weird "Studio Home" state which has become a kitchen sink of data (course list, library list, user permissions, feature flags, branding/theming). I think the idea was to improve loading times by loading all the data needed to render the studio home page from a single API, but it's just messy now. And today a few small parallel requests that are definitely needed are better than one big request that ends up loading unused data. So that should be broken up into smaller things (course list query, library list query, users permissions query, etc.) and more specific contexts.

[ormsbee]

Per @jmakowski1123, while the new screens for the course listing page could be picked up, it is unlikely to be prioritized in the Verawood timeframe. Given that, we're probably better off doing an in-place port from Redux to React Query for that page.

[bradenmacdonald]

@ormsbee Are the designs ready? There are volunteers looking for features to implement.

[kdmccormick]

Can you say more about the volunteers @bradenmacdonald ? If they are open to non-feature work, there are several frontend-app-authoring bugfixes and refactorings that we could get groomed for them.

[bradenmacdonald]

@kdmccormick Just one I've been chatting to who is more interested in feature work and the ones I saw you mention on our call today. But I expect there will be more in the future.

[kdmccormick]

@bradenmacdonald to help me get this prioritized, can you say more about the value/stratrgy of the refactoring?

• is redux slowing down development of new features?
• is redux controbuting significantly to maintenance burden or a factor in some of the existing bugs (like those in Course Authoring Frontend Bugfixes #2539)?
• is it possible to divvy this work up and do some of it for Verawood?
• is a partial completion of the refactoring valuable in itself, or does that just increase complexity?
• how much would the refactoring conflict with the Verawood libraries work? i.e., could it be done in parallel, or is it best done before or after? could it be done by a seaparate team as the libraries work, or would that cause too many cross-dependencies?

[bradenmacdonald]

@kdmccormick The only real problem with redux in principle is that it's a global state that's shared by many components and as the complexity of the application increases and you get more diverse components using the same shared state, refactors become slower because changing that shared state affects many things. So it's not really used in modern React, in favor of React Query and React builtins (state, context) which allow much more performant, localized, specialized tracking of state (and thus easier refactoring, etc.) without any drawbacks.

However, the problem with redux as implemented is that our two redux stores in this app both have a large, complex state that has no documentation, no comments, and no type safety (though I have added some very high-level, incomplete type definitions). Different parts of the code frequently make conflicting assumptions about what data is in the state and how it should be used, and this is the root cause of many small bugs. Thus, we could resolve a lot of the issues by cleaning up the redux state, adding comments and type definitions, etc. But this would still have some drawbacks as mentioned, and in particular would mean we'd still have a lot of idiosyncratic data loading code instead of just using standard (and efficient) React Query. So my preference is to rewrite things to not use redux at all, all else being equal.

Though specifically in the case of the editors code, the code is so complicated that I think the better first step is to separate each editor's state while still keeping it in Redux, completing the type definitions, and fixing the bugs, then later (likely post-Verawood) rewriting it to not use Redux (low priority). But for all the other pages where Redux is used for basic data loading and state tracking, we should just refactor it right away.

There is no conflict with the libraries work, because none of the libraries code uses Redux.

is it possible to divvy this work up and do some of it for Verawood?

Yes, just pick any part of the Redux state, find the code that uses it, convert that code to use React Query / Context / etc., then delete that part of the Redux state. It's very straightforward to split things up and you can see examples of refactoring small pieces of the redux state here:

• refactor: move 'isMarkdownEditorEnabledForContext' into EditorContext #2398
• Load waffle flags using React Query #2068

In fact, I would strongly prefer that this work be split up into many small PRs, as it makes the code so much easier to review.

is a partial completion of the refactoring valuable in itself, or does that just increase complexity?

I think almost any refactoring along these lines is valuable in itself, as it gives us type safety that we didn't have before.