Skip to content

Migrate javalabView to redux-toolkit - #51155

Merged
molly-moen merged 4 commits into
stagingfrom
molly/javalab-view-redux-toolkit
Apr 7, 2023
Merged

Migrate javalabView to redux-toolkit#51155
molly-moen merged 4 commits into
stagingfrom
molly/javalab-view-redux-toolkit

Conversation

@molly-moen

Copy link
Copy Markdown
Contributor

Migrate Java Lab view redux store to redux-toolkit format. No functionality change here.

As part of this, I updated setDisplayTheme to be a thunk instead of a reducer with a side effect (which is an anti-pattern). I ended up going with a basic thunk here since we weren't doing any management of a failed request here, just storing the new theme in redux and doing a best effort to update the theme in User Preferences. If we wanted to do something more complex, such as show an error if the request failed, createAsyncThunk may be helpful. I looked into RTK query, but it seemed like overkill for our current setup, and is a bit of a paradigm shift for thinking about redux. It may be something to revisit if we want to update our APIs.

Links

Testing story

Tested locally that updating the state still works as expected.

Follow-up work

Migrate the other parts of the Java Lab redux store to redux toolkit.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@molly-moen
molly-moen requested a review from a team April 5, 2023 22:10

@bencodeorg bencodeorg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learned a lot reading through this, very cool!

@sanchitmalhotra126 sanchitmalhotra126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@molly-moen
molly-moen merged commit 6502b26 into staging Apr 7, 2023
@molly-moen
molly-moen deleted the molly/javalab-view-redux-toolkit branch April 7, 2023 16:13
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.

3 participants