Page MenuHomePhabricator

Expand `ThreadInfo` to `LegacyThreadInfo | MinimallyEncodedThreadInfo`
ClosedPublic

Authored by atul on Jan 15 2024, 12:40 PM.
Tags
None
Referenced Files
F3528751: D10639.id.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528749: D10639.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528746: D10639.id35641.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528745: D10639.id35992.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528743: D10639.id35997.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528742: D10639.id36000.largetrue.diff
Tue, Dec 24, 10:03 AM
F3528710: D10639.id36000.diff
Tue, Dec 24, 9:54 AM
F3528709: D10639.id35997.diff
Tue, Dec 24, 9:54 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
No Lint Coverage
Unit
No Test Coverage

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.