Page MenuHomePhabricator

[lib] Add ridiculous branching to `update-thread-read-status-spec` to appease `flow`
ClosedPublic

Authored by atul on Dec 8 2023, 12:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 2:39 PM
Unknown Object (File)
Tue, Nov 5, 10:41 PM
Unknown Object (File)
Tue, Nov 5, 10:37 PM
Unknown Object (File)
Tue, Nov 5, 10:36 PM
Unknown Object (File)
Tue, Nov 5, 10:31 PM
Unknown Object (File)
Oct 23 2024, 10:33 PM
Unknown Object (File)
Oct 21 2024, 6:51 AM
Unknown Object (File)
Oct 21 2024, 2:35 AM
Subscribers
None

Details

Summary

I'm completely aware that the amount of branching and repetition here is ridiculous. This is just to help us get towards 0 flow issues, at which point I will definitely come back and do noop refactors to tidy these up.

NOTE: CI is expected to fail. I got to a point in my local environment where all flow issues were resolved, but based on earlier fedback in the stack I'm breaking them down so things are easier to review.. even if that means that intermediate steps fail.

[skip-ci]


Depends on D10270

Test Plan

CI/flow/etc.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Dec 8 2023, 12:12 PM
atul created this revision.
ashoat requested changes to this revision.Dec 11 2023, 12:23 PM

Requesting changes since I requested changes on other parts of the stack anyways, so you'll be blocked on landing. Might as well review this one in the next go-round anyways

lib/shared/updates/update-thread-read-status-spec.js
33–78 ↗(On Diff #34450)

I think this is cleaner:

const storeThreadInfo = storeThreadInfos[update.threadID];

let updatedThread;
if (storeThreadInfo.minimallyEncoded) {
  updatedThread = {
    ...storeThreadInfo,
    currentUser: {
      ...storeThreadInfo.currentUser,
      unread: update.unread,
    },
  };
} else {
  updatedThread = {
    ...storeThreadInfo,
    currentUser: {
      ...storeThreadInfo.currentUser,
      unread: update.unread,
    },
  };
}

return [
  {
    type: 'replace',
    payload: {
      id: update.threadID,
      threadInfo: updatedThread,
    },
  },
];

PS – I don't think you should leave unnecessary typehints in there if you end up needing to replace them later

This revision now requires changes to proceed.Dec 11 2023, 12:23 PM
lib/shared/updates/update-thread-read-status-spec.js
33–78 ↗(On Diff #34450)

This is much cleaner, thanks! Updating diff right now

PS – I don't think you should leave unnecessary typehints in there if you end up needing to replace them later

Type hints as in the annotations like

storeThreadInfo: RawThreadInfo

?

lib/shared/updates/update-thread-read-status-spec.js
33–78 ↗(On Diff #34450)

Yeah, that's what I mean. It's subjective... I only bring it up now since I wonder if it's cause more work for you later, having to update all of those typehints

This revision is now accepted and ready to land.Dec 14 2023, 9:44 AM