Skip to content

Commit 034c274

Browse files
mikehardymikehardy
committed
docs(new-arch): queue Phase S2 gap analysis and compare-types hints
1 parent 4c8edad commit 034c274

5 files changed

Lines changed: 90 additions & 22 deletions

File tree

‎.github/scripts/compare-types/configs/analytics.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,42 @@ const config: PackageConfig = {
100100
],
101101

102102
differentShape: [
103-
{ name: 'logEvent', reason: 'RN Firebase `logEvent` returns `Promise<void>` because Analytics calls cross the native bridge, and RN overloads omit the web `[key: string]: any` index signature on event params.' },
104-
{ name: 'setAnalyticsCollectionEnabled', reason: 'RN Firebase returns `Promise<void>` (or `Promise<null>`) because the operation crosses the native bridge, whereas the firebase-js-sdk web modular API is synchronous.' },
105-
{ name: 'setConsent', reason: 'RN Firebase returns `Promise<void>` (or `Promise<null>`) because the operation crosses the native bridge, whereas the firebase-js-sdk web modular API is synchronous.' },
106-
{ name: 'setDefaultEventParameters', reason: 'RN Firebase returns `Promise<void>` (or `Promise<null>`) because the operation crosses the native bridge, whereas the firebase-js-sdk web modular API is synchronous.' },
107-
{ name: 'setUserId', reason: 'RN Firebase returns `Promise<void>` (or `Promise<null>`) because the operation crosses the native bridge, whereas the firebase-js-sdk web modular API is synchronous.' },
108-
{ name: 'setUserProperties', reason: 'RN Firebase returns `Promise<void>` (or `Promise<null>`) because the operation crosses the native bridge, whereas the firebase-js-sdk web modular API is synchronous.' },
103+
{
104+
name: 'logEvent',
105+
reason:
106+
'RN Firebase `logEvent` returns `Promise<void>` and RN overloads omit the web `[key: string]: any` index signature on event params. ' +
107+
'Phase S hint: **Promise that could maybe sync-void+queue** (ordered native fire-and-forget; see PS-S2-gap).',
108+
},
109+
{
110+
name: 'setAnalyticsCollectionEnabled',
111+
reason:
112+
'RN Firebase returns `Promise<void>` whereas the firebase-js-sdk web modular API is synchronous. ' +
113+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
114+
},
115+
{
116+
name: 'setConsent',
117+
reason:
118+
'RN Firebase returns `Promise<void>` whereas the firebase-js-sdk web modular API is synchronous. ' +
119+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
120+
},
121+
{
122+
name: 'setDefaultEventParameters',
123+
reason:
124+
'RN Firebase returns `Promise<void>` whereas the firebase-js-sdk web modular API is synchronous. ' +
125+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
126+
},
127+
{
128+
name: 'setUserId',
129+
reason:
130+
'RN Firebase returns `Promise<void>` whereas the firebase-js-sdk web modular API is synchronous. ' +
131+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
132+
},
133+
{
134+
name: 'setUserProperties',
135+
reason:
136+
'RN Firebase returns `Promise<void>` whereas the firebase-js-sdk web modular API is synchronous. ' +
137+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
138+
},
109139
{ name: 'Analytics', reason: 'RN Firebase extends the Analytics service interface with native bridge methods (collection toggles, predefined event helpers, and iOS on-device conversion measurement) that are not on the firebase-js-sdk web service type.' },
110140
{ name: 'ConsentSettings', reason: 'RN Firebase maps most consent flags to native boolean toggles, while the firebase-js-sdk uses `ConsentStatusString` for web gtag consent modes.' },
111141
{ name: 'Currency', reason: 'RN Firebase narrows Analytics currency values to `number` for native event payloads, while the firebase-js-sdk also allows string currency codes for web gtag events.' },

‎.github/scripts/compare-types/configs/app-check.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ const config: PackageConfig = {
8989
{
9090
name: 'initializeAppCheck',
9191
reason:
92-
'RN Firebase returns `Promise<AppCheck>` because App Check initialization crosses the native bridge, whereas the firebase-js-sdk returns `AppCheck` synchronously.',
92+
'RN Firebase returns `Promise<AppCheck>` whereas the firebase-js-sdk returns `AppCheck` synchronously. ' +
93+
'Phase S hint: **Promise that could maybe sync-void+queue** after provider/refresh state is installed synchronously (see PS-S2-gap).',
9394
},
9495
{
9596
name: 'AppCheckOptions',

‎.github/scripts/compare-types/configs/firestore.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -273,26 +273,25 @@ const config: PackageConfig = {
273273
name: 'deleteAllPersistentCacheIndexes',
274274
reason:
275275
'Returns `Promise<void>` in RN Firebase vs `void` in the firebase-js-sdk. ' +
276-
'The RN implementation is async because it delegates to the native module.',
276+
'Phase S hint: **keep-async: deferred persistent-cache IO** (web void schedules disk/index work; see PS-S2-gap).',
277277
},
278278
{
279279
name: 'disablePersistentCacheIndexAutoCreation',
280280
reason:
281281
'Returns `Promise<void>` in RN Firebase vs `void` in the firebase-js-sdk. ' +
282-
'The RN implementation is async because it delegates to the native module.',
282+
'Phase S hint: **keep-async: deferred persistent-cache IO** (see PS-S2-gap).',
283283
},
284284
{
285285
name: 'enablePersistentCacheIndexAutoCreation',
286286
reason:
287287
'Returns `Promise<void>` in RN Firebase vs `void` in the firebase-js-sdk. ' +
288-
'The RN implementation is async because it delegates to the native module.',
288+
'Phase S hint: **keep-async: deferred persistent-cache IO** (see PS-S2-gap).',
289289
},
290290
{
291291
name: 'initializeFirestore',
292292
reason:
293-
'Returns `Promise<Firestore>` in RN Firebase vs `Firestore` in the ' +
294-
'firebase-js-sdk. The RN implementation is async because it initialises ' +
295-
'the native Firestore module.',
293+
'Returns `Promise<Firestore>` in RN Firebase vs `Firestore` in the firebase-js-sdk. ' +
294+
'Phase S hint: **Promise that could maybe sync-void+gate** (in-memory settings visible before return; see PS-S2-gap).',
296295
},
297296
{
298297
name: 'onSnapshotsInSync',

‎.github/scripts/compare-types/configs/storage.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,16 @@ const config: PackageConfig = {
5454
reason:
5555
'RN Firebase-specific function for setting the maximum retry time for ' +
5656
'non-upload/download operations on Android and iOS. The firebase-js-sdk ' +
57-
'exposes this as a writable property on the `FirebaseStorage` instance.',
57+
'exposes this as a writable property on the `FirebaseStorage` instance. ' +
58+
'Phase S hint: **Promise that could maybe sync-void+queue** (sync JS field + native setter; see PS-S2-gap).',
5859
},
5960
{
6061
name: 'setMaxUploadRetryTime',
6162
reason:
6263
'RN Firebase-specific function for setting the maximum upload retry time ' +
6364
'on Android and iOS. The firebase-js-sdk exposes this as a writable property ' +
64-
'on the `FirebaseStorage` instance.',
65+
'on the `FirebaseStorage` instance. ' +
66+
'Phase S hint: **Promise that could maybe sync-void+queue** (see PS-S2-gap).',
6567
},
6668
{
6769
name: 'setMaxDownloadRetryTime',
@@ -144,9 +146,9 @@ const config: PackageConfig = {
144146
name: 'UploadTask',
145147
reason:
146148
'RN Firebase returns `Promise<boolean>` from `cancel()`, `pause()`, and ' +
147-
'`resume()` to communicate asynchronously with the native iOS/Android modules, ' +
148-
'whereas the firebase-js-sdk returns a synchronous `boolean`. Error callback ' +
149-
'types also use `NativeFirebaseError` instead of `StorageError`.',
149+
'`resume()` whereas the firebase-js-sdk returns a synchronous `boolean`. ' +
150+
'Error callback types also use `NativeFirebaseError` instead of `StorageError`. ' +
151+
'Phase S hint: **convert after native fix** (sync boolean from native task state; see PS-S2-gap).',
150152
},
151153
{
152154
name: 'EmulatorMockTokenOptions',

‎okf-bundle/new-architecture/migration-work-queue.md‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ timestamp: 2026-06-26T00:00:00Z
88

99
# TurboModule migration — work queue
1010

11-
> **IN PROGRESS (2026-07-03):** Phase **S** — async→sync conversion review before merge. Phases 0–5, Docs/PD, and R are complete.
11+
> **IN PROGRESS (2026-07-03):** Phase **S** — `convert` inventory complete; **S2** native-change follow-up queued ([§ Phase S2](#phase-s2-native-change-follow-up-gap-analysis-2026-07-03)). Phases 0–5, Docs/PD, and R are complete.
1212
> **Goal/order:** app foundation → hard probe → easy wins → remaining complex → sync conversion → coordinated break → cleanup (events, shared-state encapsulation). Decisions: [architecture-decisions.md](architecture-decisions.md). Links: [implementation workflow](turbomodule-implementation-workflow.md), [change authoring](../testing/change-authoring-workflow.md), [functions reference](../../../packages/functions/) ([PR #8603](https://github.com/invertase/react-native-firebase/pull/8603)).
1313
1414
Ephemeral tracker; see [OKF policy](../documentation-policy.md).
@@ -176,7 +176,7 @@ Pick **one** of `firestore` or `auth` in Phase 1 (firestore = multi-module + pip
176176
| **PD** | Platform-divergence documentation | **done** | multi-package JSDoc/docs — [§ Phase PD](#phase-pd-platform-divergence-documentation) |
177177
| **4** | Remaining complex | **done** | `firestore`, `messaging`, `database`, `auth` |
178178
| **5** | Android-only / misc | **done** | `phone-number-verification` |
179-
| **S** | Sync conversion (forced-async → sync) | **in progress** (gap-analysis complete; first slice `app/registerVersion`); prereq **S0** complete ([interruption batch](#interruption-batch-standalone-commits)) | All migrated packages — [§ sync conversion](#phase-s-sync-conversion-forced-async--sync) |
179+
| **S** | Sync conversion (forced-async → sync) | **convert done**; **S2** queued ([§ Phase S2](#phase-s2-native-change-follow-up-gap-analysis-2026-07-03)); prereq **S0** complete ([interruption batch](#interruption-batch-standalone-commits)) | [§ sync conversion](#phase-s-sync-conversion-forced-async--sync) |
180180
| **R** | Pre-merge full validation | **done** | Revert harness narrowing; remove `NativeModules` fallback + throw test ([§ Phase R additions](#phase-r-additions)); [full tier](../testing/running-e2e.md#e2e-validation-tiers-unit-focused-area-focused-full) 3-platform before coordinated major |
181181
| **C** | EventEmitter cleanup | deferred | All — [§ deferred cleanup](#deferred-cleanup-phase-eventemitter) |
182182
| **E** | Shared-state encapsulation | deferred (optional) | `app` + readers — [§ Phase E](#phase-e-shared-state-encapsulation-optional) |
@@ -233,6 +233,41 @@ The "keep async if it does network/IO/disk" rule in the discriminator **assumes*
233233

234234
**Next slice:** `convert` inventory complete (2026-07-03). Remaining gap-analysis verdicts (`needs-native-change`, `keep-async`) are out of Phase S scope until native behavior is clarified or documented only.
235235

236+
### Phase S2: native-change follow-up (gap-analysis 2026-07-03)
237+
238+
Deep read-only analysis of **`needs-native-change`** and **`keep-async`** candidates after the `convert` inventory shipped. Canonical evidence table for discussion and future implementation slices. Compare-types `differentShape` entries for these methods carry **Phase S hint** phrases (short pointers only — this table holds the real state).
239+
240+
**Hint vocabulary (compare-types annotations):**
241+
242+
| Phrase | Meaning |
243+
|--------|---------|
244+
| *(none — entry removed)* | Phase S `convert` complete; no async-vs-sync delta |
245+
| **Promise that could maybe sync-void+queue** | Web is sync/fire-and-forget; RN may return void after sync local state update + ordered background native dispatch |
246+
| **Promise that could maybe sync-void+gate** | Web returns sync instance/state; RN must not return until in-memory settings visible to all later native entry points (or gate ops until background work completes) |
247+
| **convert after native fix** | Native shell close to sync; fix semantics first (e.g. boolean return), then drop Promise |
248+
| **keep-async: deferred persistent-cache IO** | Web `void` schedules real disk/index work; do not block JS; Promise may mean dispatch-only today |
249+
250+
**Executive summary:** Sync return while queueing background work is valid only when RNFB updates the same **synchronous observable state** web updates before return, or when the web API is truly fire-and-forget with no ordering dependency. Queue-only is insufficient when the next line of user code assumes the effect is visible (`initializeFirestore` settings, analytics ordering, upload retry times at task construction). Firestore persistent cache index manager methods are **`keep-async`**: web `void` schedules persistent-cache work; **`sync-void+gate`** would need a native completion signal RNFB does not expose today.
251+
252+
| Method | Package | compare:types | Web SDK (sync state before return?) | RNFB today | Verdict | Recommended path | Risks if forced sync void | Notes |
253+
|--------|---------|---------------|-------------------------------------|------------|---------|------------------|---------------------------|-------|
254+
| `logEvent` | analytics | `differentShape` | `void`; async pipeline; init/measurement ordering internal | Android `Tasks.call`; iOS direct | needs-native-change → sync-void+queue | Ordered fire-and-forget queue or direct native if SDK proven cheap | Unordered queue races with setters | Web does not expose completion |
255+
| `setAnalyticsCollectionEnabled` | analytics | `differentShape` | `void`; ga-disable after init | Android `Tasks.call`; iOS direct | needs-native-change → sync-void+queue | Same ordered path as events | Events logged with old collection flag | |
256+
| `setConsent` | analytics | `differentShape` | Sync init stash or immediate gtag | Android `Tasks.call`; iOS direct | needs-native-change → sync-void+queue | Serial queue shared with analytics calls | Immediate logEvent uses old consent | |
257+
| `setDefaultEventParameters` | analytics | `differentShape` | Sync init stash or immediate gtag | Android `Tasks.call`; iOS direct | needs-native-change → sync-void+queue | **Clearest ordering requirement** | Next event misses defaults | |
258+
| `setUserId` / `setUserProperties` | analytics | `differentShape` | Init-ordered or immediate gtag | Android `Tasks.call`; iOS direct | needs-native-change → sync-void+queue | Same analytics queue | Stale id/properties on next event | Android SDK source dive for direct-call safety |
259+
| RNFB-only analytics helpers | analytics | `extraInRN` | N/A | Same native path | Not Phase S parity | Align if core analytics goes fire-and-forget | Public API break without web signal | Out of compare-types completion criteria |
260+
| `initializeAppCheck` | app-check | `differentShape` | Sync provider/state install; token fetch background | JS awaits native configure | needs-native-change → sync-void+queue | Sync return after provider/refresh state installed | getToken/listeners race before provider ready | Native provider install source dive |
261+
| `initializeFirestore` | firestore | `differentShape` | Returns instance; lazy config but ordered internally | JS awaits settings(); native prefs write async | needs-native-change → sync-void+gate | In-memory settings registry visible before return | Immediate writes/listeners with old settings | Queue-only insufficient |
262+
| `UploadTask.pause/resume/cancel` | storage | `UploadTask` `differentShape` | Sync state machine + boolean | Android bool via Promise; iOS bool path incomplete | convert after native fix | Sync boolean from native task state | Lying about pause/cancel state | iOS shell review needed |
263+
| `setMaxOperationRetryTime` / `setMaxUploadRetryTime` | storage | `extraInRN` + `FirebaseStorage` shape | Sync in-memory field update | JS field then native resolve | sync-void+queue / convert | Sync void; native before return | Upload task captures stale retry at construction | Upload retry stronger ordering than operation retry |
264+
| `setMaxDownloadRetryTime` | storage | `extraInRN` | N/A (RN-only) | Same pattern | Not parity | Consistency if siblings change | RN-only API | |
265+
| `enablePersistentCacheIndexAutoCreation` | firestore | `differentShape` | `void`; schedules persistent-cache work | Native resolves immediately; no completion hook | **keep-async** | Keep Promise or document dispatch-only | Queries/listeners race index config | |
266+
| `disablePersistentCacheIndexAutoCreation` | firestore | `differentShape` | same | same | **keep-async** | same | Stale auto-creation until SDK finishes | |
267+
| `deleteAllPersistentCacheIndexes` | firestore | `differentShape` | `void` + deferred disk/index work | same | **keep-async** | Do not Phase S convert | Immediate queries use old indexes | sync-void+gate needs native completion signal |
268+
269+
**Suggested implementation slices (when resumed):** PS-S2-storage-tasks (closest to convert) → PS-S2-analytics-queue → PS-S2-app-check-init → PS-S2-firestore-init; PS-S2-firestore-index = document/keep-async only.
270+
236271
---
237272

238273
## Phase E: shared-state encapsulation (optional)
@@ -332,9 +367,9 @@ Skip steps 1–2 when spec shape is known (most Tier D packages).
332367

333368
**Label:** `phase-s-sync-conversion` (2026-07-03)
334369

335-
**Next item:** Phase **S** `convert` inventory **complete** — deferred `needs-native-change` / `keep-async` items documented in gap-analysis table
370+
**Next item:** Phase **S2** native-change follow-up — analysis queued; pick slice when resumed ([§ Phase S2](#phase-s2-native-change-follow-up-gap-analysis-2026-07-03))
336371

337-
**Current gates:** PS-database-online all gates **closed** — committing `refactor(database): return sync parity for connection controls`.
372+
**Current gates:** Phase S `convert` inventory **complete** (four commits on `new-architecture`). PS-S2-gap analysis **done** — implementation deferred.
338373

339374
**Host rule:** one `:test-cover` at a time — never parallel subagents with e2e.
340375

@@ -376,6 +411,7 @@ Skip steps 1–2 when spec shape is known (most Tier D packages).
376411
| Phase S auth parser/TOTP sync parity | PS-auth-parsers | **closed** | **closed** | **closed** | done | `area-focused` | `refactor(auth): return sync parity for auth parsers` | Committed 2026-07-03. Sync `isSignInWithEmailLink` + `TotpSecret.generateQrCodeUrl`; native sync error shape; jest.setup; tests + e2e sync calls; compare-types + v26 doc. Validation: Jest 91/91, compare:types, lint, reference:api, codegen:verify exit 0. Android area e2e 159/15/0 (`/tmp/rnfb-e2e-android-auth-phaseS-final.log`). |
377412
| Phase S perf metric controls sync parity | PS-perf-metrics | **closed** | **closed** | **closed** | done | `area-focused` | `refactor(perf): return sync parity for metric controls` | Sync trace/http/screen start/stop; Android *Sync helpers; iOS sync shells. Jest 10/10; compare:types, lint, reference:api green. Review e2e iOS/Android 60/2/0 (`/tmp/rnfb-e2e-*-perf-review.log`). macOS N/A (module skip). Remediation: perf usage docs await removed. |
378413
| Phase S database connection sync parity | PS-database-online | **closed** | **closed** | **closed** | done | `area-focused` | `refactor(database): return sync parity for connection controls` | Sync turbo + instance goOnline/goOffline; Android direct SDK calls; e2e await removed on modular calls. Jest 52/52; codegen:verify exit 0. Review e2e macOS 181/10/0, iOS 182/9/0, Android 183/8/0 (`/tmp/rnfb-e2e-*-database-review.log`). |
414+
| Phase S2 native-change gap-analysis | PS-S2-gap | n/a | n/a | n/a | done | `none` | none | Completed 2026-07-03. Full table [§ Phase S2](#phase-s2-native-change-follow-up-gap-analysis-2026-07-03); compare-types hint phrases updated. Implementation slices deferred (storage tasks closest). |
379415

380416
---
381417

0 commit comments

Comments
 (0)