Details

Reviewers

tomek
atul
• jacek
ashoat

Commits

rCOMM33daa5a36d6d: Remove unnecessary (dead code) code from message reducer.

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

marcin created this revision.Jul 12 2022, 7:41 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJul 12 2022, 7:41 AM

Harbormaster completed remote builds in B10465: Diff 14410.Jul 12 2022, 7:53 AM

marcin requested review of this revision.Jul 12 2022, 7:53 AM

ashoat requested changes to this revision.Jul 12 2022, 12:14 PM

ashoat added inline comments.

lib/reducers/message-reducer.js
1270–1271 ↗	(On Diff #14410)	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 ↗	(On Diff #14410)	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

ashoat added inline comments.Jul 12 2022, 12:17 PM

lib/reducers/message-reducer.js
1270–1271 ↗	(On Diff #14410)	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)

Harbormaster completed remote builds in B10507: Diff 14455.Jul 14 2022, 3:12 AM

marcin added a child revision: D4508: Filter local multimedia messages.Jul 14 2022, 4:35 AM

marcin added 1 blocking reviewer(s): atul.Jul 14 2022, 5:45 AM

ashoat accepted this revision.Jul 14 2022, 6:43 AM

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