Skip to content

Commit 296466d

Browse files
committed
refactor: cleanup legacy arch native module fallback
1 parent 392fb77 commit 296466d

6 files changed

Lines changed: 90 additions & 35 deletions

File tree

jest.setup.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,8 @@ jest.doMock('react-native', () => {
2626
// make sure PlatformConstants is visible otherwise turbo modules default init fails
2727
ReactNative.NativeModules['PlatformConstants'] = {};
2828

29-
return Object.setPrototypeOf(
30-
{
31-
Platform: {
32-
OS: 'android',
33-
select: () => {},
34-
},
35-
TurboModuleRegistry: {
36-
get: jest.fn(() => undefined),
37-
getEnforcing: jest.fn(() => {
38-
throw new Error('TurboModuleRegistry.getEnforcing: module not found');
39-
}),
40-
},
41-
AppRegistry: {
42-
registerHeadlessTask: jest.fn(),
43-
},
44-
NativeModules: {
45-
...ReactNative.NativeModules,
29+
const turboModuleLookup: Record<string, unknown> = {
30+
...ReactNative.NativeModules,
4631
NativeRNFBTurboAnalytics: {
4732
logEvent: jest.fn(),
4833
setAnalyticsCollectionEnabled: jest.fn(),
@@ -646,7 +631,28 @@ jest.doMock('react-native', () => {
646631
list: jest.fn(() => Promise.resolve({ items: [], prefixes: [], pageToken: null })),
647632
listAll: jest.fn(() => Promise.resolve({ items: [], prefixes: [], pageToken: null })),
648633
},
634+
};
635+
636+
return Object.setPrototypeOf(
637+
{
638+
Platform: {
639+
OS: 'android',
640+
select: () => {},
641+
},
642+
TurboModuleRegistry: {
643+
get: jest.fn((moduleName: string) => turboModuleLookup[moduleName]),
644+
getEnforcing: jest.fn((moduleName: string) => {
645+
const mod = turboModuleLookup[moduleName];
646+
if (!mod) {
647+
throw new Error('TurboModuleRegistry.getEnforcing: module not found');
648+
}
649+
return mod;
650+
}),
651+
},
652+
AppRegistry: {
653+
registerHeadlessTask: jest.fn(),
649654
},
655+
NativeModules: turboModuleLookup,
650656
},
651657
ReactNative,
652658
);

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

Lines changed: 11 additions & 11 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-02):** Phase **5** complete (P5pnv + auth fix committed). All native packages migrated except Phase **R** / **S** / **C**. Next: Phase **R** pre-merge validation or opportunistic docs.
11+
> **IN PROGRESS (2026-07-02):** Phase **R** — remove `NativeModules` fallback (PR-fallback). Phases 0–5 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).
@@ -169,15 +169,15 @@ Pick **one** of `firestore` or `auth` in Phase 1 (firestore = multi-module + pip
169169
| **1** | Hard probe | **done** | `firestore` (multi-module + pipelines; NewArch-AD-14a composite) |
170170
| **2** | Easy wins | **done** | `installations`, `perf`, `in-app-messaging`, `app-distribution`, `ml` |
171171
| **3** | Moderate | **done** | `app-check`, `remote-config`, `analytics`, `crashlytics`, `storage` |
172-
| **NB** | Interruption batch — JS infra/test/chore (standalone commits) | queued | `app` resolver refactor+perf, shared contract-test helper, harness snapshot; `compare-types` **S0**[§ interruption batch](#interruption-batch-standalone-commits) |
173-
| **3.5** | Guardrails — spec↔native parity + codegen-drift CI (**gates 4**) | queued | all migrated packages — [§ Phase 3.5](#phase-35-guardrails) |
172+
| **NB** | Interruption batch — JS infra/test/chore (standalone commits) | **done** | `app` resolver refactor+perf, shared contract-test helper, harness snapshot; `compare-types` **S0**[§ interruption batch](#interruption-batch-standalone-commits) |
173+
| **3.5** | Guardrails — spec↔native parity + codegen-drift CI (**gates 4**) | **done** | all migrated packages — [§ Phase 3.5](#phase-35-guardrails) |
174174
| **4a** | messaging event decision — gap-analysis (**gates 4**) | **done** | `messaging`[§ Phase 4a](#phase-4a-messaging-event-decision) |
175175
| **Docs** | New-Architecture requirements + migration-doc consolidation | queued (opportunistic) | docs — [§ Phase Docs](#phase-docs-new-architecture-requirements-and-doc-consolidation) |
176176
| **PD** | Platform-divergence documentation | queued (opportunistic) | multi-package JSDoc/docs — [§ Phase PD](#phase-pd-platform-divergence-documentation) |
177-
| **4** | Remaining complex | queued | other Tier A/B + `messaging`, `database` |
178-
| **5** | Android-only / misc | queued | `phone-number-verification` |
177+
| **4** | Remaining complex | **done** | `firestore`, `messaging`, `database`, `auth` |
178+
| **5** | Android-only / misc | **done** | `phone-number-verification` |
179179
| **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) |
180-
| **R** | Pre-merge full validation | queued | 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 |
180+
| **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) |
183183

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

323323
## Current snapshot
324324

325-
**Label:** `phase-5-pnv` (2026-07-02); **harness:** full (implementer applies area narrowing for e2e)
325+
**Label:** `phase-r-fallback` (2026-07-02); **harness:** full (no narrowing for final review)
326326

327-
**Next item:** Phase **R** pre-merge validation or opportunistic PDoc
327+
**Next item:** Phase **R** PR-fallback — remove `NativeModules` fallback
328328

329-
**Current gates:** P5pnv all gates **closed** — committed 2026-07-02. Auth fix `fix(auth): rename delete turbo method for C++ codegen compatibility` committed separately.
329+
**Current gates:** PR-fallback all gates **closed** — committed as `refactor: cleanup legacy arch native module fallback`. Proof run + review green.
330330

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

@@ -358,11 +358,11 @@ Skip steps 1–2 when spec shape is known (most Tier D packages).
358358
| Phase 4a messaging event decision | P4a | n/a | n/a | n/a | done | `none` | none | **Defer** events to Phase C — [NewArch-AD-4 § messaging](architecture-decisions.md#messaging--defer-eventemitter-cutover-migrate-shell-only-in-phase-4). |
359359
| 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). |
360360
| 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. |
361-
| Phase 4 `auth` TurboModules | P4u | **closed** | **closed** | **closed** | done | `area-focused` | `feat(auth)!: migrate auth to TurboModules` | **Follow-up (uncommitted):** spec `delete` → C++ keyword collision; rename + regen; `TaskExecutorService(TAG)` fix applied on disk. |
361+
| 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`. |
362362
| 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`. |
363363
| Phase Docs — NA reqs + consolidation | PDoc | open | open | open | documentation | `none` | `docs: new-architecture requirements + migration consolidation` | Opportunistic; does not gate 4. |
364364
| Phase PD — platform divergence | PPD | open | open | open | documentation | `none` | `docs(<pkg>): …` per package | Opportunistic; gap-analysis first. |
365-
| Phase R — remove `NativeModules` fallback | PR-fallback | open | open | open | pre-merge-validation | `full` | (Phase R) | Decision B. Jest + `app` e2e "unknown module throws". |
365+
| 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. |
366366

367367
---
368368

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { describe, expect, it, jest } from '@jest/globals';
2+
import { TurboModuleRegistry } from 'react-native';
3+
import { getReactNativeModule } from '../lib/internal/nativeModuleAndroidIos';
4+
5+
describe('getReactNativeModule (NewArch-AD-6 Phase R)', function () {
6+
it('throws when TurboModuleRegistry has no module for the name', function () {
7+
const unknownModule = 'NativeRNFBTurboNonExistentModule';
8+
jest.mocked(TurboModuleRegistry.get).mockReturnValueOnce(null);
9+
10+
expect(() => getReactNativeModule(unknownModule)).toThrow(
11+
`Native module ${unknownModule} is not registered.`,
12+
);
13+
});
14+
});
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright (c) 2016-present Invertase Limited & Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this library except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
*/
17+
import { getReactNativeModule } from '@react-native-firebase/app/dist/module/internal/nativeModule';
18+
19+
describe('getReactNativeModule resolver', function () {
20+
it('throws for an unknown module name', function () {
21+
const unknownModule = 'NativeRNFBTurboNonExistentModule';
22+
23+
try {
24+
getReactNativeModule(unknownModule);
25+
throw new Error('did not throw for unknown module');
26+
} catch (e) {
27+
e.message.should.containEql(unknownModule);
28+
e.message.should.containEql('not registered');
29+
}
30+
});
31+
});

packages/app/lib/internal/nativeModuleAndroidIos.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable no-console */
2-
import { NativeModules, TurboModuleRegistry } from 'react-native';
2+
import { TurboModuleRegistry } from 'react-native';
33

44
const DYNAMIC_CONSTANT_KEYS = new Set(['androidPlayServices']);
55

@@ -42,16 +42,20 @@ function withTurboConstants(
4242
}
4343

4444
/**
45-
* Unified native module resolver — TurboModule first, legacy NativeModules fallback.
45+
* Unified native module resolver — TurboModule only (NewArch-AD-6 Phase R).
4646
*/
47-
export function getReactNativeModule(moduleName: string): Record<string, unknown> | undefined {
47+
export function getReactNativeModule(moduleName: string): Record<string, unknown> {
4848
const turboModule = TurboModuleRegistry.get(moduleName);
49+
if (!turboModule) {
50+
throw new Error(`Native module ${moduleName} is not registered.`);
51+
}
52+
4953
const nativeModule = withTurboConstants(
5054
moduleName,
51-
(turboModule ?? NativeModules[moduleName]) as Record<string, unknown> | undefined,
52-
);
55+
turboModule as Record<string, unknown>,
56+
) as Record<string, unknown>;
5357

54-
if (!globalThis.RNFBDebug || !nativeModule) {
58+
if (!globalThis.RNFBDebug) {
5559
return nativeModule;
5660
}
5761

tests/ios/Podfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2606,7 +2606,7 @@ SPEC CHECKSUMS:
26062606
RNFBApp: 893629fc75425937059b0f64e58ec9d5a066aefc
26072607
RNFBAppCheck: 8ba8648a565a2c9c5f057906323475bba79c1a6a
26082608
RNFBAppDistribution: 5fa4b64e4e62ffca3af9851a293b64ac801d11c1
2609-
RNFBAuth: 6656cfdec1c2c7a238666ab46f00f37650ea4e86
2609+
RNFBAuth: e24099af4c7db560ecdc07817f8fafaecbf2fa92
26102610
RNFBCrashlytics: 14eea95d1f7669287597f1e27602abecc5699a91
26112611
RNFBDatabase: a74a6e18a24749fcdfdfd9234e494da996a22511
26122612
RNFBFirestore: 7b2480c59acae62bb9607cdf28c1e0e157641288

0 commit comments

Comments
 (0)