[lib] avoid modifying state on rehydrate action
AbandonedPublic
Actions

Authored by kamil on Feb 2 2024, 3:07 AM.

Details

Reviewers

Summary

According to docs: "Auto merge means if the some piece of substate was modified by your reducer during the REHYDRATE action, it will skip this piece of state." src.

If the logic changes and we modify keyserverStore state on persist/REHYDRATE it could cause skipping the rehydrated state.

Test Plan

Add this line
keyserverStore = { ...keyserverStore }; in if - previously it was causing logging out on each refresh, now it shouldn't

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Feb 2 2024, 3:07 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 2 2024, 3:07 AM

kamil edited the summary of this revision. (Show Details)Feb 2 2024, 3:07 AM

kamil added a child revision: D10934: [lib] refactor keyserver-related actions in `master-reducer` to ops.Feb 2 2024, 3:09 AM

kamil published this revision for review.Feb 2 2024, 3:29 AM

Harbormaster completed remote builds in B26464: Diff 36582.Feb 2 2024, 3:35 AM

I don't understand the point of this diff. Do we need to do keyserverStore = { ...keyserverStore }; for some reason?
I know what mechanism you mean, with the REHYDRATE skipping a part of state if it was changed by the reducer, but this code should never change anything on REHYDRATE action, because keyserverStore is not changed by the REHYDRATE action. So the inner ifs (line 102 and 115) will always return false and not execute. And it is very intentional that the keyserver store is not changed on the REHYDRATE action.

If the logic changes and we modify keyserverStore state on persist/REHYDRATE it could cause skipping the rehydrated state.

If the logic changes and we modify keyserverStore state on persist/REHYDRATE the rehydration WILL be skipped for the keyserver store, REGARDELESS of this code here. On the other hand, if we decide to change keyserverStore on REHYDRATE action, then maybe this code should run? Are we sure it shouldn't?

This revision now requires changes to proceed.Feb 5 2024, 1:57 AM

Discussed this with @inka today in the office - we think this code is confusing but adding action.type !== rehydrateActionType is not making it any better.

Revision Contents
Changeset List

Path

Size

lib/

reducers/

master-reducer.js

9 lines

Diff 36582

View Options

lib/reducers/master-reducer.js

[lib] avoid modifying state on rehydrate actionAbandonedPublicActions