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

[Tech/Demo] Migrate Wine Manager status state to Zustand #3428

Merged
merged 2 commits into from
May 17, 2024

Conversation

CommandMC
Copy link
Collaborator

This is a proof-of-concept of using Zustand (https://github.com/pmndrs/zustand) for one of our many states-to-be-managed

I don't know how much I have to explain here, the code looks very straight-forward to me, but it's probably confusing in some places if you're not used to the syntax. Please ask questions! With that said, a general explanation:

First off, we've had somewhat of a code split here before (with some state being updated by the Frontend and some by the Backend). This would work just fine now as well, but I thought I might as well move everything over to the Backend while I'm at it. Thus, the installWineVersion and removeWineVersion IPC callbacks no longer return anything, instead settings state directly themselves

On the Frontend side, we first create a store to hold our state (in our case, the easiest structure is a Record, mapping wine version names to their state), and then give the Backend an option to update this state using the (already present but now somewhat modified) progressOfWineManager Frontend message.
The WineItem components then listen to their respective version's state updates (using useShallow to really only re-render when the specific version changes). Everything else is then handled by Zustand and React


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

State logic in general was sort-of handled by the Backend and sort-of
handled by the Frontend. This was changed to now only be the Backend's
job
@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label Jan 11, 2024
@CommandMC CommandMC requested a review from arielj January 11, 2024 00:04
@CommandMC CommandMC self-assigned this Jan 11, 2024
Other than the one required change (at the end), I've also removed the
dependence of the current working directory being the repo's origin
(which makes it possible to run just this one test directly in IDEs)
Copy link
Collaborator

@arielj arielj left a comment

Choose a reason for hiding this comment

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

this looks pretty good to me, this already solves the problem I had with the frontend losing the wine installation progress status temporarily while components are mounted

I like that the migration can be done really granular

@CommandMC CommandMC force-pushed the ref/state-management-zustand-test branch from 3655105 to 8d88c48 Compare March 11, 2024 17:00
@CommandMC CommandMC force-pushed the ref/state-management-zustand-test branch from 8d88c48 to d7c2b19 Compare April 9, 2024 15:29
@CommandMC CommandMC force-pushed the ref/state-management-zustand-test branch from d7c2b19 to 99125d2 Compare May 15, 2024 20:40
@arielj arielj merged commit 55a54e2 into main May 17, 2024
9 checks passed
@arielj arielj deleted the ref/state-management-zustand-test branch May 17, 2024 03:40
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants