Differential D9453
[lib] Update `reduceThreadActivity` to handle `processUpdatesActionType` and others Authored by atul on Oct 11 2023, 1:27 PM. Tags None Referenced Files
Details To address @tomek's point here: https://phab.comm.dev/D9402#276831 Went ahead and added all the actionTypes for that could include ClientUpdateInfo. @tomek wanted to get your thoughts on this approach, or if I should just add processUpdatesActionType. Added an additional filter check to see if there were any DELETE_THREAD updates before creating a new state object as an optimization since presumably this action handler branch will be hit more times now. Careful reading + flow, not sure if further testing is worthwile.
Diff Detail
Event TimelineComment Actions Agree it is safer to handle all the actions that include the updates. But, at the same time, it is possible to introduce a new action that contains updates and forget to add it here. We probably should figure out a different approach and use it in all the places - e.g. instead of checking action.type, check if it has updateRedult.newUpdates (not sure if Flow will like it). - It isn't blocking this diff, but is a separate follow-up. One issue with the approach from this diff is that it is a step backward in terms of https://linear.app/comm/issue/ENG-4244/update-redux-state-based-on-updates. What we should do instead is to introduce a new function to update specs and call it here. Comment Actions
Comment Actions Makes sense - we can make this a follow-up. The idea of update specs is quite simple - if some code in reducers depends on the update type, instead of having ifs, we should create a function in a spec for the corresponding type and select this function from update-specs object by a type. Comment Actions @atul can you make a Linear task to track refactoring this to use the new update specs approach, and link the task here before landing? | ||||||||||||||||||||||||||||||||||||||||