Page MenuHomePhabricator

Remove unnecessary (dead code) code from message reducer.
ClosedPublic

Authored by marcin on Jul 12 2022, 7:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 6:04 PM
Unknown Object (File)
Sat, Nov 2, 10:33 AM
Unknown Object (File)
Oct 26 2024, 2:26 PM
Unknown Object (File)
Oct 24 2024, 6:54 PM
Unknown Object (File)
Oct 24 2024, 6:44 PM
Unknown Object (File)
Oct 24 2024, 6:36 PM
Unknown Object (File)
Oct 24 2024, 4:35 PM
Unknown Object (File)
Oct 24 2024, 3:27 PM

Details

Summary

This differential removes code that was used to modify messageStore on rehydrateActionType. Now that we do not persist messages in messageStore in redux, messages are always {} on rehydrateActionType, so the code never get executed.

Test Plan

This differential does not change app functionality, but only the way it works. So we expect the app to work exactly the same after this change.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-1350
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat requested changes to this revision.Jul 12 2022, 12:14 PM
ashoat added inline comments.
lib/reducers/message-reducer.js
1270–1271

It makes sense to remove this... it isn't doing anything anymore, since messages will be empty.

But how are we going to handle filtering out local-only multimedia messages going forward?

native/redux/persist.js
384–386

Confused why this change was combined with the message-reducer.js change. In the diff description you say:

Now we want to remove this code, but we still need messageStore to have this field to avoid null/undefined errors.

Does specifying this rehydrate transform address this somehow? What's confusing to me is that the code in message-reducer.js is basically still doing the same thing it was before... so it doesn't make sense that removing that code requires adding this new code here.

Note that I left a comment on Linear with some more questions.

This revision now requires changes to proceed.Jul 12 2022, 12:14 PM
lib/reducers/message-reducer.js
1270–1271

Ah, looks like it was brought back in D4508

Remove rehydreate action transform since it is not necessary - it was a mistake to think it was needed.

marcin retitled this revision from REmove unnecessary code from message reducer. add rehydrate transform to insert messages field to messageStore since it is essential to avoid null/undefined bugs to Remove unnecessary (dead code) code from message reducer..Jul 14 2022, 3:01 AM
marcin edited the summary of this revision. (Show Details)
ashoat added inline comments.
lib/reducers/message-reducer.js
1269 ↗(On Diff #14455)

Should we just remove rehydrateActionType here? It's not strictly necessary in a reducer... if it does nothing, it seems best to remove it

Looks good, thanks for catching this.

lib/reducers/message-reducer.js
1269 ↗(On Diff #14455)

Agree with this, would reduce clutter in already

This revision is now accepted and ready to land.Jul 14 2022, 9:06 PM
lib/reducers/message-reducer.js
1269 ↗(On Diff #14455)

*Agree with this, would reduce clutter in already large file

Remove redundant rehydrateActionType handling

Rebase master before landing