[native] Introduce `assertAllThreadInfosAreLegacy` to address migration 29 `flow` issues
ClosedPublic
Actions

Authored by atul on Dec 8 2023, 1:54 PM.

Details

Reviewers

ashoat
ginsu
tomek
rohan

Commits

rCOMM4f8b105b534f: [native] Introduce `assertAllThreadInfosAreLegacy` to address migration 29…

Summary

We can safely assume that when older migrations are run that all of the RawThreadInfos will be LegacyRawThreadInfos and we don't need to worry about MinimallyEncodedRawThreadInfos. So, instead of updating all of those migration functions to accomodate MinimallyEncodedRawThreadInfos, we can instead use assertAllThreadInfosAreLegacy to assert that we'll always be passing LegacyRawThreadInfos to them.

We basically go through every RawThreadInfo and ensure that it's a LegacyRawThreadInfo by asserting that the minimallyEncoded field is missing. Then, at the callsite (migration 29), we immediately cast back to:

{
  +[id: string]: LegacyRawThreadInfo,
}

NOTE: Discussed an alternative approach with @ashoat that allows us to avoid any-casting. Will put up rest of the diffs in this stack, then: 1. Either update this diff in place 2. Introduce follow-up diff

[skip-ci]

Depends on D10280

Test Plan

CI/flow/etc.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 25049
Build 218737: arc lint + arc unit

Event Timeline

atul requested review of this revision.Dec 8 2023, 1:54 PM

atul created this revision.

Harbormaster completed remote builds in B24927: Diff 34461.Dec 8 2023, 1:54 PM

atul added inline comments.Dec 8 2023, 1:54 PM

native/redux/persist.js
384 ↗	(On Diff #34461)	Snuck this simple type name change

Current status:

web: 0 issues
lib: 0 issues
keyserver: 0 issues
native: 28 issues (all in `persist.js), regarding just 3 migrations:

atul added a child revision: D10283: [lib] Update `convertThreadStoreThreadInfosToNewIDSchema` migration to ensure `rawThreadInfos` are NOT minimally encoded.Dec 10 2023, 3:01 PM

ashoat requested changes to this revision.Dec 11 2023, 11:14 AM

ashoat added inline comments.

lib/shared/thread-utils.js

1760–1771 ↗

(On Diff #34461)

For other reviewers' context, my suggestion was:

I think you could skip the any if you rewrite it as a map operation that returns each individual threadinfo after the invariant

This seems to work:

function assertAllThreadInfosAreLegacy(rawThreadInfos: RawThreadInfos): {
  [id: string]: LegacyRawThreadInfo,
} {
  return Object.fromEntries(
    Object.entries(rawThreadInfos).map(([id, rawThreadInfo]) => {
      invariant(
        !rawThreadInfo.minimallyEncoded,
        `rawThreadInfos shouldn't be minimallyEncoded`,
      );
      return [id, rawThreadInfo];
    }),
  );
}