[lib] Update `convertThreadStoreThreadInfosToNewIDSchema` migration to ensure…
24bd20b41376
Actions

Description

[lib] Update convertThreadStoreThreadInfosToNewIDSchema migration to ensure rawThreadInfos are NOT minimally encoded

Summary:
convertThreadStoreThreadInfosToNewIDSchema calls the utility function convertRawThreadInfoToNewIDSchema where both the input and output is any.

convertThreadStoreThreadInfosToNewIDSchema is called from createUpdateDBOpsForThreadStoreThreadInfos which expects a migrationFunc: RawThreadInfos => RawThreadInfos.

Rather than updating convertThreadStoreThreadInfosToNewIDSchema and all of those re-usable migration utils to support this legacy migration, I typed convertThreadStoreThreadInfosToNewIDSchema as accepting RawThreadInfos and have an invariant that enforces that we'll never actually pass a minimallyEncodedRawThreadInfo through this function.

NOTE: I think this might be one of the more "controversial" changes I'm making to flow issues. It's kind of a lazy approach since I'm assuming that it's reasonable to assume convertRawThreadInfoToNewIDSchema won't be used in new migrations... so we're "lying" to flow by saying it accepts MinimallyEncodedRawThreadInfos and then immediately failing with an invariant if one is actually passed through. I still think this is cleaner than some of the other refactoring approaches I tried which required more involved changes to eg createUpdateDBOpsForThreadStoreThreadInfos. Would love to hear thoughts from @ashoat, @tomek, @michal if there are any approaches that would be better.

[skip-ci]

Depends on D10281

Test Plan: Flow/CI/etc. I think in an ideal world we'd have unit tests for migrations to ensure that changes to the utility functions we consume in them don't lead to older migrations changing, but for now I just thought through it and read carefeully.

Reviewers: ashoat, ginsu, tomek, rohan

Reviewed By: ashoat

Subscribers: ashoat, tomek, michal

Differential Revision: https://phab.comm.dev/D10283