Skip to content

Commit 4850938

Browse files
committed
refactor(app): return sync parity for registerVersion
1 parent 46cf354 commit 4850938

5 files changed

Lines changed: 32 additions & 18 deletions

File tree

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,6 @@ const config: PackageConfig = {
128128
reason:
129129
'Returns `Promise<ReactNativeFirebase.FirebaseApp>` because initialization crosses the native bridge. Accepts `ReactNativeFirebase.FirebaseAppOptions` and optional `ReactNativeFirebase.FirebaseAppConfig` (name / auth domain) instead of firebase-js-sdk `(FirebaseOptions, string)` only.',
130130
},
131-
{
132-
name: 'registerVersion',
133-
reason:
134-
'Returns `Promise<void>` in RN Firebase vs `void` in firebase-js-sdk. Bridge-forced async today — Phase S sync-conversion candidate when native work is in-memory only.',
135-
},
136131
{
137132
name: 'setLogLevel',
138133
reason:

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

Lines changed: 20 additions & 9 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):** Phases **Docs** + **PD** committed. Next: opportunistic Phase **S** / **C** / merge handoff.
11+
> **IN PROGRESS (2026-07-03):** Phase **S** — async→sync conversion review before merge. 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).
@@ -26,7 +26,7 @@ Durable architectural decisions are owned by **[architecture-decisions.md](archi
2626
| [NewArch-AD-3](architecture-decisions.md#newarch-ad-3--strong-codegen-typing--accepted) | Strong Codegen typing |
2727
| [NewArch-AD-4](architecture-decisions.md#newarch-ad-4--events-deferred-to-phase-c--accepted) | Events deferred to [Phase C](#deferred-cleanup-phase-eventemitter) |
2828
| [NewArch-AD-5](architecture-decisions.md#newarch-ad-5--commit-generated-code--accepted) | Commit generated code |
29-
| [NewArch-AD-6](architecture-decisions.md#newarch-ad-6--unified-native-module-resolver--accepted) | Unified resolver (`TurboModuleRegistry.get ?? NativeModules`) |
29+
| [NewArch-AD-6](architecture-decisions.md#newarch-ad-6--unified-native-module-resolver--accepted) | Unified resolver; Phase R final state is TurboModule-only (no `NativeModules` fallback) |
3030
| [NewArch-AD-7](architecture-decisions.md#newarch-ad-7--codegenconfigname--aggregate-library-name-one-codegenconfig-per-package--accepted) | `codegenConfig.name` = `RNFB<Package>TurboModules` (all packages) |
3131
| [NewArch-AD-8](architecture-decisions.md#newarch-ad-8--turbomodule-js-enumeration-forin--objectcreate--accepted) | Enumerate with `for...in` + `Object.create` |
3232
| [NewArch-AD-9](architecture-decisions.md#newarch-ad-9--requiresmainqueuesetup-returns-no--accepted) | `requiresMainQueueSetup` returns `NO` |
@@ -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) | queued (scope open — [NewArch-AD-16](architecture-decisions.md#newarch-ad-16--phase-s-asyncsync-conversion--open)); prereq **S0** ([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) | **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) |
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) |
@@ -203,7 +203,7 @@ Pick **one** of `firestore` or `auth` in Phase 1 (firestore = multi-module + pip
203203

204204
This is a **coordinated public-API change** (async→sync is observable to consumers) and ships in the same major as the migration — see [implementation workflow § Phase S](turbomodule-implementation-workflow.md#phase-s-sync-conversion-forced-async--sync).
205205

206-
### Gap-analysis (deferred — capture only, do not size yet)
206+
### Gap-analysis (completed 2026-07-03)
207207

208208
The "keep async if it does network/IO/disk" rule in the discriminator **assumes** that where firebase-js-sdk is synchronous, the work is genuinely in-memory and non-blocking. That assumption is **unverified** and is the core thing the gap-analysis must establish. Open question to resolve:
209209

@@ -223,7 +223,15 @@ The "keep async if it does network/IO/disk" rule in the discriminator **assumes*
223223

224224
**Candidate sources:** the `compare:types` async-vs-sync entries **plus** a manual sweep of `Promise`-returning methods whose firebase-js-sdk equivalent is sync but which `compare:types` does not flag (e.g. unregistered packages or utils-only exports). The third verdict (`needs-native-change`) is the interesting one the user raised: cases where we are async only because our native implementation chose IO, not because the operation requires it.
225225

226-
**Defer the actual inventory** — this section is the brief for it.
226+
**Inventory result (2026-07-03):**
227+
228+
| Verdict | Candidate(s) | Notes |
229+
|---------|--------------|-------|
230+
| `convert` | `app/registerVersion`; `auth/isSignInWithEmailLink`; `auth/TotpSecret.generateQrCodeUrl`; `perf` trace/http/screen `start`/`stop`; likely `database` `goOnline`/`goOffline` instance surface | Highest confidence: web/native behavior is parser, string construction, cached object, or in-memory SDK state. |
231+
| `needs-native-change` | `analytics` fire-and-forget setters/events; `app-check/initializeAppCheck`; `firestore/initializeFirestore`; `storage` task controls; storage retry setters | Web APIs are sync or fire-and-forget, but RNFB native paths currently wrap async work or need behavior clarification before sync conversion. |
232+
| `keep-async` | Firestore persistent cache index manager delete/enable/disable | Web returns `void` but starts real persistent-cache work; do not block JS thread. |
233+
234+
**Next slice:** `app/registerVersion` because RNFB already behaves synchronously and Phase S should only change the public type/compare-types entry. Planned subject: `refactor(app): return sync parity for registerVersion`.
227235

228236
---
229237

@@ -322,11 +330,11 @@ Skip steps 1–2 when spec shape is known (most Tier D packages).
322330

323331
## Current snapshot
324332

325-
**Label:** `phase-docs-pd` (2026-07-03)**complete**
333+
**Label:** `phase-s-sync-conversion` (2026-07-03)
326334

327-
**Next item:** Phase **S** (async→sync) or merge / coordinated major handoff
335+
**Next item:** Phase **S** auth parser/TOTP URL sync conversion implementation
328336

329-
**Current gates:** PDoc + PPD all gates **closed** — committed `docs: document all API gaps / platform divergence / migration items` (remediation amended: analytics no-op semantics, messaging AD-4 JSDoc breadth, logTransaction platform note).
337+
**Current gates:** Phase **S** `app/registerVersion` gates **closed**; next pickup is auth parser/TOTP URL sync conversion.
330338

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

@@ -359,10 +367,13 @@ Skip steps 1–2 when spec shape is known (most Tier D packages).
359367
| Phase 4 `messaging` TurboModules | P4m | **closed** | **closed** | **closed** | done | `area-focused` | `feat(messaging)!: migrate messaging to TurboModules` | Turbo shell only (AD-4 event path preserved). Jest 38/38 + parity 32/32; codegen:verify incl messaging; legacy Java removed. iOS/Android area e2e green on method-call + listener-registration scope. Foreground `onMessage` delivery test **left `xit`** — flaky FCM harness; re-enable in [Phase C](#deferred-cleanup-phase-eventemitter). |
360368
| Phase 4 `database` TurboModules | P4d | **closed** | **closed** | **closed** | done | `area-focused` | `feat(database)!: migrate database to TurboModules` | 5 Turbo specs/shells + committed codegen; Jest 49 + parity 38; area e2e macOS 213 / iOS 219 / Android 220 (delta logs `/tmp/rnfb-e2e-*-database-delta-review.log`). Remediation: invalidate/teardown parity (Reference+Transaction), onComplete guard, contract test 22 methods, jest mock cleanup. Orchestration: macOS/Android `:8090` pre-flight in `firebase.test.js` + OKF. **Deferred minor:** void spec vs Promise JS API; silent Query `RejectedExecutionException` post-invalidate. |
361369
| Phase 4 `auth` TurboModules | P4u | **closed** | **closed** | **closed** | done | `area-focused` | `feat(auth)!: migrate auth to TurboModules` | 60-method spec + shells; review e2e macOS 171 / iOS 185 / Android 192. Follow-up committed: `fix(auth): rename delete turbo method for C++ codegen compatibility`. |
362-
| Phase 5 `phone-number-verification` TurboModules | P5pnv | **closed** | **closed** | **closed** | done | `area-focused` | `feat(phone-number-verification)!: migrate phone-number-verification to TurboModules` | Android-only 6-method spec; review e2e Android 40/2 pending (log `/tmp/rnfb-e2e-android-pnv-review.log`). AD-18 E10 direct resolver. Parity registered. **Bundled auth fix:** separate commit `fix(auth): rename delete turbo method for C++ codegen compatibility`. |
370+
| Phase 5 `phone-number-verification` TurboModules | P5pnv | **closed** | **closed** | **closed** | done | `area-focused` | `feat(phone-number-verification)!: migrate phone-number-verification to TurboModules` | Android-only 6-method spec; AD-18 E10 direct resolver. Parity registered; later full Android Phase R proof covered the package path. |
363371
| Phase Docs — NA reqs + consolidation | PDoc | **closed** | **closed** | **closed** | done | `none` | `docs: document all API gaps / platform divergence / migration items` | v26 NA section + platform table; lint/reference:api green. |
364372
| Phase PD — platform divergence | PPD | **closed** | **closed** | **closed** | done | `none` | `docs: document all API gaps / platform divergence / migration items` | 19 pkg usage notes + JSDoc @remarks; serious review findings remediated in same commit. |
365373
| Phase R — remove `NativeModules` fallback | PR-fallback | **closed** | **closed** | **closed** | done | `full` + RNFBDebug | `refactor: cleanup legacy arch native module fallback` | Proof 683/823/849 (`/tmp/rnfb-e2e-phaseR-proof-*.log`). Review 2026-07-03: no critical/serious. Android cold boot committed separately. |
374+
| Phase S gap-analysis | PS-gap | n/a | n/a | n/a | done | `none` | none | Completed 2026-07-03. First implementation slice: `app/registerVersion`; follow-ups: auth parser/TOTP URL, then perf metric controls. |
375+
| Phase S `app/registerVersion` sync parity | PS-app-registerVersion | **closed** | **closed** | **closed** | done | `area-focused` | `refactor(app): return sync parity for registerVersion` | Implemented 2026-07-03: `registerVersion(): void`, sync throw Jest assertion, removed stale `configs/app.ts` async-vs-sync entry. Green: `lerna:prepare`, `tsc:compile`, `tsc:compile:consumer`, focused app Jest 10/10, `reference:api`, `compare:types`, `lint:js`. Independent review: no findings; no e2e needed for type-only scope. |
376+
| Phase S auth parser/TOTP sync parity | PS-auth-parsers | **open** | **open** | **open** | `implementation` | `unit-focused` | `refactor(auth): return sync parity for auth parsers` | Candidate methods: `isSignInWithEmailLink` and `TotpSecret.generateQrCodeUrl`; gap-analysis classified both as parser/string-only with cached native SDK objects. |
366377

367378
---
368379

packages/app/__tests__/app.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ describe('App', function () {
2121
expect(registerVersion).toBeDefined();
2222
});
2323

24+
it('`registerVersion` throws synchronously on react-native', function () {
25+
expect(() => registerVersion('test-library', '1.0.0')).toThrow(
26+
'registerVersion is only supported on Web',
27+
);
28+
});
29+
2430
it('`onLog` function is properly exposed to end user', function () {
2531
expect(onLog).toBeDefined();
2632
});

packages/app/__tests__/specNativeParityHelper.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ export type SpecParityContextEntry = {
4444
iosHeaderPath: string;
4545
};
4646

47-
/** Explicit spec/native paths for packages with non-default codegen layout (see database/messaging guardrail commits). */
47+
/**
48+
* Explicit spec/native paths for packages with non-default codegen layout (see
49+
* database/messaging guardrail commits).
50+
*/
4851
export const PACKAGE_SPEC_PARITY_CONTEXTS: Partial<
4952
Record<MigratedTurboPackage, readonly SpecParityContextEntry[]>
5053
> = {
@@ -54,8 +57,7 @@ export const PACKAGE_SPEC_PARITY_CONTEXTS: Partial<
5457
specPath: 'packages/auth/specs/NativeRNFBTurboAuth.ts',
5558
androidSpecPath:
5659
'packages/auth/android/src/main/java/io/invertase/firebase/auth/generated/java/com/facebook/fbreact/specs/NativeRNFBTurboAuthSpec.java',
57-
iosHeaderPath:
58-
'packages/auth/ios/generated/RNFBAuthTurboModules/RNFBAuthTurboModules.h',
60+
iosHeaderPath: 'packages/auth/ios/generated/RNFBAuthTurboModules/RNFBAuthTurboModules.h',
5961
},
6062
],
6163
'phone-number-verification': [

packages/app/lib/modular.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function registerVersion(
4848
_libraryKeyOrName: string,
4949
_version: string,
5050
_variant?: string,
51-
): Promise<void> {
51+
): void {
5252
throw new Error('registerVersion is only supported on Web');
5353
}
5454

0 commit comments

Comments
 (0)