66 lines
4.2 KiB
Markdown
66 lines
4.2 KiB
Markdown
|
# Code standardization & modernization review (2026-07-06)
|
||
|
|
|
||
|
|
A grounded pass over the Android app (313 Kotlin files), Cloud Functions (TypeScript), and build. Each
|
||
|
|
item is a *real, measured* inconsistency (with counts), not generic advice. Applied = done + verified
|
||
|
|
this session; the rest are prioritized by value ÷ risk so the riskier ones can be greenlit deliberately.
|
||
|
|
|
||
|
|
## Applied (done + verified)
|
||
|
|
|
||
|
|
1. **Compose state collection unified on `collectAsStateWithLifecycle`** — was 55 files on
|
||
|
|
`collectAsState()` vs 2 on the lifecycle-aware variant; now all 57. Added
|
||
|
|
`androidx.lifecycle:lifecycle-runtime-compose`. Upstream flow collection pauses when the UI isn't
|
||
|
|
visible. Commit `61a8609`.
|
||
|
|
2. **Firebase Tasks → `.await()` (safe subset)** — `FirestoreOutcomeDataSource` (mixed both styles in
|
||
|
|
one file), `FirestoreCoupleDataSource`, `PlayIntegrityChecker`. Commit `7507ca6`.
|
||
|
|
|
||
|
|
## Recommended next — LOW risk, high value
|
||
|
|
|
||
|
|
3. **Finish Firebase Tasks → `.await()`** across the remaining ~11 data sources
|
||
|
|
(`FirestoreAnswerDataSource` 11 blocks, `FirestoreUserDataSource` 13, `FirebaseAuthDataSource` 13,
|
||
|
|
`FirestoreConversationDataSource`, `…DateSwipe/DatePlan/DateReflection/DateMemory/QuestionThread/
|
||
|
|
BucketList/DateMatch`, `FirebaseStorageDataSource`). ~72 blocks remain. **Per-block review required**
|
||
|
|
— these files mix `callbackFlow` snapshot listeners (leave as-is) and one transaction
|
||
|
|
(`FirestoreDateMatchDataSource`) with the convertible one-shot Tasks. Watch for the failure→null
|
||
|
|
idiom (see `PlayIntegrityChecker.verifyWithServer`: use `runCatching{ …await() }.getOrNull()`, not a
|
||
|
|
bare `await()`). Not unit-tested (live Firebase I/O), so do it incrementally with a smoke per file,
|
||
|
|
not a bulk sed.
|
||
|
|
|
||
|
|
## Recommended — MEDIUM risk / churn
|
||
|
|
|
||
|
|
4. **Gradle version catalog (`gradle/libs.versions.toml`)** — 57 dependency versions are hardcoded
|
||
|
|
across the build files. The modern Gradle standard centralizes versions + enables the `libs.*`
|
||
|
|
accessors. Build-only, fully verifiable (build must pass), but pure mechanical churn with typo risk.
|
||
|
|
5. **`FirebaseFirestore`/`FirebaseFunctions` injected directly into 9 ViewModels/screens**
|
||
|
|
(`HomeViewModel`, `DailyQuestionViewModel`, `QuestionThreadViewModel`, `BucketListViewModel`,
|
||
|
|
`DateBuilderViewModel`, `MessagesInboxViewModel`, `ConversationViewModel`, `PartnerHomeScreen`,
|
||
|
|
`DeleteAccountScreen`) — an architectural leak past the repository/data-source layer that the rest of
|
||
|
|
the app respects. `HomeViewModel` even runs raw Firestore `addSnapshotListener`s. Worth extracting to
|
||
|
|
data sources, but higher-risk (touches live listeners) and best done one ViewModel at a time.
|
||
|
|
|
||
|
|
## Recommended — HIGHER risk, do deliberately
|
||
|
|
|
||
|
|
6. **~175 hardcoded `Color(0xFF…)` literals in `ui/`** vs. theme tokens. A theme-token migration is the
|
||
|
|
right modern Compose practice, but carries visual-regression risk; the repo already has
|
||
|
|
`scripts/theme-scan.sh` and prior brand work, so pair any migration with that scanner + screenshot
|
||
|
|
checks. Do per-screen, not en masse.
|
||
|
|
7. **Cloud Functions on the v1 API** (`functions.https.onCall`, `functions.pubsub.schedule`, all 30
|
||
|
|
imports `from 'firebase-functions'`). Firebase Functions **v2** (`firebase-functions/v2/*`) is the
|
||
|
|
modern target (better cold-start, concurrency, typed params). Migration changes signatures + deploy
|
||
|
|
config and touches the billing webhook / callables — meaningful risk; do as its own project with
|
||
|
|
sandbox verification, not bundled.
|
||
|
|
8. **11 `: any` types in TypeScript** (e.g. `onCall(async (data: any, …))`) — tighten to typed request
|
||
|
|
shapes. Low risk, low urgency; fold into the v2 migration if/when it happens.
|
||
|
|
|
||
|
|
## Healthy already (no action)
|
||
|
|
|
||
|
|
- **Material3 only** (86 files, zero Material2 imports).
|
||
|
|
- **No `GlobalScope`**; scoping via `viewModelScope`/`lifecycleScope` throughout.
|
||
|
|
- Only **9 `!!`** non-null assertions app-wide.
|
||
|
|
- Analytics hashing single-sourced (`AnalyticsHashing`, this session); DI split of `@Binds`/`@Provides`
|
||
|
|
modules is idiomatic (not an inconsistency).
|
||
|
|
|
||
|
|
## Suggested order
|
||
|
|
|
||
|
|
3 (finish `.await()`) → 4 (version catalog) → 5 (ViewModel Firebase extraction, per-VM) → 7 (Functions
|
||
|
|
v2, standalone) → 6 (colors, per-screen) → 8 (TS types, with #7).
|