Page MenuHomePhabricator

Expand `ThreadInfo` to `LegacyThreadInfo | MinimallyEncodedThreadInfo`
ClosedPublic

Authored by atul on Jan 15 2024, 12:40 PM.
Tags
None
Referenced Files
F1800951: D10639.largetrue.diff
Mon, May 20, 6:56 AM
F1800950: D10639.id.largetrue.diff
Mon, May 20, 6:56 AM
F1800949: D10639.id35641.largetrue.diff
Mon, May 20, 6:56 AM
F1800947: D10639.id35992.largetrue.diff
Mon, May 20, 6:56 AM
F1800946: D10639.id35997.largetrue.diff
Mon, May 20, 6:56 AM
F1800945: D10639.id36000.largetrue.diff
Mon, May 20, 6:55 AM
F1800895: D10639.id36000.diff
Mon, May 20, 6:52 AM
F1800893: D10639.id35997.diff
Mon, May 20, 6:52 AM
Subscribers
None

Details

Summary

Part of the work described here: https://linear.app/comm/issue/ENG-6366/rename-minimallyencodedrawthreadinfo-to-rawthreadinfo

Used IDE "Inline type alias" feature to do this refactor automatically. Read through to make sure there were no issues. Had to insert ? manually for MinimallyEncodedThreadInfo where it was previously ?ThreadInfo.


Depends on D10638

Test Plan

flow

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

NOTE: Still need to make some tweaks to thread-utils and thread-selectors before removing ThreadInfo altogether. Will be handled in followup diff.
atul requested review of this revision.Jan 15 2024, 1:01 PM

This is going to add a lot of noise to the codebase... what % of these unions do you think you'll be able to "back out" back to just the minimally-encoded variant?

web/types/nav-types.js
1 ↗(On Diff #35641)

Interesting that this file got random ESLint/Prettier changes

This revision is now accepted and ready to land.Jan 15 2024, 5:41 PM

This is going to add a lot of noise to the codebase... what % of these unions do you think you'll be able to "back out" back to just the minimally-encoded variant?

In practice it ended up being ~75+%. They're pretty much fully removed from web/native client except to support old migrations.

Unless you're talking about git history noise vs the noise of having multiple types around?

web/types/nav-types.js
1 ↗(On Diff #35641)

Looks like we also should have a newline after @flow, will update this diff.