Skip to content

Commit f1b59d4

Browse files
mikehardymikehardy
committed
fix: deep PR review and related fixes for type gaps
1 parent d325ed0 commit f1b59d4

17 files changed

Lines changed: 984 additions & 123 deletions

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

Lines changed: 52 additions & 58 deletions
Large diffs are not rendered by default.

‎compare_types_triage.md‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
# Auth modular API — compare:types triage
2+
3+
Working document for sorting known differences between `@react-native-firebase/auth` and the firebase-js-sdk modular Auth API.
4+
5+
**Related files**
6+
7+
- [`.github/scripts/compare-types/configs/auth.ts`](.github/scripts/compare-types/configs/auth.ts) — machine-readable registry (`yarn compare:types auth`)
8+
- [`docs/migrating-to-v25.mdx`](docs/migrating-to-v25.mdx) — user-facing migration guide (also read by agents)
9+
- [`packages/auth/lib/modular.ts`](packages/auth/lib/modular.ts) — modular API
10+
- [`packages/auth/lib/web/RNFBAuthModule.ts`](packages/auth/lib/web/RNFBAuthModule.ts) — Other platform js-sdk bridge
11+
- [`tests/local-tests/`](tests/local-tests/) — mini-app exercising MFA / TOTP on Other platforms
12+
13+
---
14+
15+
## Platform contexts
16+
17+
| Context | Examples | Backend | DOM |
18+
|---------|----------|---------|-----|
19+
| **iOS/Android** | iOS simulator, Android device | Native Firebase Auth SDK | No |
20+
| **Other/Hermes** | react-native-macos, Windows RN | firebase-js-sdk via JS bridge | No |
21+
| **Other/Web** | Browser tab, web embedding | firebase-js-sdk | Yes |
22+
| **Other/All** | Other/Hermes **and** Other/Web | firebase-js-sdk | varies |
23+
24+
`isOther` = `Platform.OS !== 'ios' && Platform.OS !== 'android'`.
25+
26+
**Note:** `yarn compare:types` compares **TypeScript shapes** only. Runtime behaviour gaps (throws vs works, `auth.config` empty object) may be documented in triage / migration docs even when signatures match.
27+
28+
---
29+
30+
## Won't change now (#1–#2, #5, #7–#8, #11, #37)
31+
32+
Permanent on iOS/Android, or verified elsewhere; no further work this round.
33+
34+
| # | Item | Why |
35+
|---|------|-----|
36+
| **1** | `isSignInWithEmailLink` async | Native bridge — `Promise<boolean>` vs js-sdk sync `boolean` |
37+
| **2** | `TotpSecret.generateQrCodeUrl` async | Native bridge — `Promise<string>` vs js-sdk sync `string` |
38+
| **5** | `useUserAccessGroup` | iOS native keychain only |
39+
| **7** | `PhoneAuthState`, `PhoneAuthListener`, `PhoneAuthSnapshot`, `PhoneAuthError` | Native listener types (iOS/Android) |
40+
| **8** | `PhoneAuthProvider` MF overloads | Native MFA bridge extension on iOS/Android. MFA on **Other** works via js-sdk (`tests/local-tests`); not missing Other work |
41+
| **11** | Credential `token` / `secret` bridge fields | Native module wire format |
42+
| **37** | `PhoneAuthProvider` compare:types entry | Same as **#8** — documents intentional RN extension, not stale |
43+
44+
---
45+
46+
## Update compare:types note (#3–#4, #6, #9–#10, #12–#22, #23a–#23h)
47+
48+
Won't change on **iOS/Android** now, but `configs/auth.ts` reasons should document future **Other/Hermes**, **Other/Web**, or **Other/All** support. Registry updated unless noted.
49+
50+
| # | Item | compare:types note (summary) |
51+
|---|------|------------------------------|
52+
| **3** | `verifyPhoneNumber` | iOS/Android only. **Other/All:** use `signInWithPhoneNumber` / js-sdk `PhoneAuthProvider` — not this listener API. |
53+
| **4** | `setLanguageCode` | **Other/All:** js-sdk `languageCode` / `useDeviceLanguage` delegation possible. |
54+
| **6** | `getCustomAuthDomain` | iOS/Android native helper. **Other/All:** js-sdk `auth.config` may cover this — see **#32**. |
55+
| **9** | `ApplicationVerifier` ignored | iOS/Android+Hermes ignore. **Other/Web:** js-sdk `RecaptchaVerifier` possible. |
56+
| **10** | `OAuthProvider` scopes / `toObject()` | iOS/Android: `toObject()` for native bridge. **Other/All:** scopes/params via js-sdk; `toObject()` stays bridge-only. |
57+
| **12** | `connectAuthEmulator` `disableWarnings` | iOS/Android: `emulatorConfig` only. **Other/Web:** DOM banner suppression possible. |
58+
| **13** | `signInWithRedirect` / `linkWithRedirect` | iOS/Android: immediate `UserCredential`. **Other/Web:** js-sdk redirect flow possible. |
59+
| **14** | `reauthenticateWithRedirect` | iOS/Android: `Promise<void>`. **Other/Web:** js-sdk redirect semantics possible. |
60+
| **15** | `getRedirectResult` | iOS/Android: throws. **Other/Web:** js-sdk delegation possible. *(Types match js-sdk; runtime note only — not in `differentShape`.)* |
61+
| **16** | `setPersistence` | iOS/Android: throws. **Other/All:** js-sdk persistence possible. *(Types match; runtime note only.)* |
62+
| **17** | `useDeviceLanguage` | iOS/Android: throws. **Other/All:** js-sdk possible. *(Types match; runtime note only.)* |
63+
| **18** | `revokeAccessToken` | iOS/Android: throws. **Other/All:** js-sdk likely possible. *(Types match; runtime note only.)* |
64+
| **19** | `linkWithPhoneNumber` | iOS/Android: throws. **Other/Web:** js-sdk + reCAPTCHA possible; Hermes maybe in emulator. *(Types match; runtime note only.)* |
65+
| **20** | `reauthenticateWithPhoneNumber` | Same as **#19**. |
66+
| **21** | `initializeAuth(deps)` ignored | iOS/Android: deps ignored. **Other/All:** js-sdk persistence/error-map deps possible. *(Types match; runtime note only.)* |
67+
| **22** | `credentialFromResult` / `credentialFromError` → `null` | Signatures match js-sdk on all providers. **Runtime:** always `null` today. **iOS/Android:** no native extraction planned. **Other/Hermes:** not delegated. **Other/Web:** future work — delegate to firebase-js-sdk in `RNFBAuthModule` (see **#40**). Documented on `OAuthProvider`, `FacebookAuthProvider`, and `PhoneAuthProvider` in `configs/auth.ts`; same runtime applies to `GoogleAuthProvider`, `GithubAuthProvider`, `TwitterAuthProvider`. |
68+
| **23a** | `initializeRecaptchaConfig` | **Other/Web** only. |
69+
| **23b** | `RecaptchaVerifier` | **Other/Web** only. |
70+
| **23c** | `SAMLAuthProvider` | **Other/Web** only. |
71+
| **23d** | `AuthErrorCodes` | **Other/All** re-export possible. |
72+
| **23e** | `browser*Persistence` | **Other/Web** only. |
73+
| **23f** | `browserPopupRedirectResolver` | **Other/Web** only. |
74+
| **23g** | `debugErrorMap` / `prodErrorMap` | **Other/All** via `initializeAuth`. |
75+
| **23h** | `inMemoryPersistence` / `ReactNativeAsyncStorage` | **Other/Hermes** primarily via `initializeAuth`. |
76+
77+
---
78+
79+
## RN extensions — documented, intentional (#8, #30, #33, #38, #39)
80+
81+
| # | Item | compare:types treatment |
82+
|---|------|------------------------|
83+
| **8** | `PhoneAuthProvider` MF overloads | `differentShape` — native MFA bridge only |
84+
| **30** | `FacebookAuthProvider.credential(token, secret)` | `differentShape` — limited-login secret overload (js-sdk public API is single-arg) |
85+
| **33** | `UserCredential.additionalUserInfo` + `AdditionalUserInfoNative` | `differentShape` on `UserCredential`; `extraInRN` for `AdditionalUserInfoNative` type |
86+
| **38** | `TotpMultiFactorGenerator` auth overload | `differentShape` — non-default native Firebase apps |
87+
| **39** | `TotpSecret.openInOtpApp` | `differentShape` — RN-only deep-link helper |
88+
89+
### #33 — `additionalUserInfo` (resolved)
90+
91+
**Decision:** Modular sign-in helpers attach **enumerable** `additionalUserInfo` on `UserCredential` when the native bridge returns it.
92+
93+
- Core fields match firebase-js-sdk: `isNewUser`, `profile`, `providerId`, `username`.
94+
- Extra native keys are **spread** onto the object (reflection/copy from bridge payload) for backwards compatibility.
95+
- `getAdditionalUserInfo(userCredential)` returns the same normalized object (`AdditionalUserInfo | null` in declarations).
96+
- Export `AdditionalUserInfoNative` (`AdditionalUserInfo & Record<string, unknown>`) when callers need to type provider-specific native extras.
97+
98+
firebase-js-sdk keeps `additionalUserInfo` off the public `UserCredential` interface and exposes it via `getAdditionalUserInfo` only — RNFB documents the extra property as an intentional extension.
99+
100+
---
101+
102+
## Planned — deprecation v25 (#24–#27)
103+
104+
| # | Item | Status | Migration |
105+
|---|------|--------|-----------|
106+
| **24** | `AppleAuthProvider` | `@deprecated` + migration guide | `new OAuthProvider('apple.com').credential({ idToken, rawNonce })` — verified in `provider.e2e.js` |
107+
| **25** | `OIDCAuthProvider` | `@deprecated` + migration guide | `new OAuthProvider('oidc.<suffix>').credential({ idToken, accessToken })` |
108+
| **26** | `OIDCProvider` interface | `@deprecated` on type | With **#25** |
109+
| **27** | `FirebaseAuthTypes` namespace | Ongoing deprecation | Modular root imports |
110+
111+
---
112+
113+
## Document only — no code change now (#32, #36, #40)
114+
115+
### #32 — `auth.config` (option B)
116+
117+
| Platform | Runtime |
118+
|----------|---------|
119+
| iOS/Android | Always `{}` (native SDKs don't expose web config) |
120+
| Other/All | js-sdk can populate `auth.config` — **not delegated today** |
121+
122+
**Decision:** Keep unified `Auth.config` type (firebase-js-sdk shape). Document runtime split in migration guide; narrow per-platform types deferred.
123+
124+
### #36 — namespaced `sendSignInLinkToEmail` defaults
125+
126+
**Not** an iOS vs Other split — **namespaced vs modular** within RNFB:
127+
128+
| API | `actionCodeSettings` |
129+
|-----|------------------------|
130+
| **Modular** `sendSignInLinkToEmail(auth, email, settings)` | **Required** (matches firebase-js-sdk) |
131+
| **Namespaced** `firebase.auth().sendSignInLinkToEmail(email, settings?)` | Optional — `_resolveActionCodeSettings()` fills `url` from `app.options.authDomain` and defaults `handleCodeInApp: true` |
132+
133+
**Decision:** Document only. Namespaced convenience for native apps; modular stays strict.
134+
135+
### #40 — `credentialFromResult` / `credentialFromError`
136+
137+
Overlap **#22**. Types match firebase-js-sdk; runtime always returns `null` on all platforms today.
138+
139+
| Platform | Today | Future (if any) |
140+
|----------|-------|-----------------|
141+
| **iOS/Android** | Always `null` | No native bridge investment planned |
142+
| **Other/Hermes** | Always `null` | Unlikely — js-sdk popup/redirect credential recovery needs DOM context |
143+
| **Other/Web** | Always `null` | Delegate to firebase-js-sdk in `RNFBAuthModule` |
144+
145+
Documented in **#22**, provider JSDoc, `configs/auth.ts` (`OAuthProvider`, `FacebookAuthProvider`, `PhoneAuthProvider`), and the migration guide.
146+
147+
---
148+
149+
## Done (#28–#31, #33)
150+
151+
| # | Item | What changed |
152+
|---|------|--------------|
153+
| **28** | `ActionCodeURL.parseLink` / `parseActionCodeURL` | Pure JS port from firebase-js-sdk; **sync** `ActionCodeURL \| null` on all platforms |
154+
| **29** | Provider credential return types | Emit `OAuthCredential` class name (not `OAuthCredentialType` alias) in provider static methods |
155+
| **31** | Provider `differentShape` cleanup | Removed stale type-only provider entries where declarations now match |
156+
| **33** | `additionalUserInfo` | Enumerable on modular `UserCredential`; native extras preserved; `AdditionalUserInfoNative` exported |
157+
| **24–27** | Provider deprecations | JSDoc + [`docs/migrating-to-v25.mdx`](docs/migrating-to-v25.mdx) |
158+
159+
**#32 implementation** (narrow `auth.config` typing per platform) — **deferred**; see Document only above.
160+
161+
---
162+
163+
## Triage complete (Auth v25 type gap)
164+
165+
All **#1–#40** items are classified. `yarn compare:types auth` passes with documented differences only.
166+
167+
| Outcome | Items |
168+
|---------|-------|
169+
| **Won't change** | #1, #2, #5, #7, #8, #11, #37 |
170+
| **Implemented** | #28, #29, #31, #33, #24–#27 |
171+
| **Document only** | #32, #36, #40 (+ runtime-only **#15–#22** where types already match) |
172+
| **Deferred implementation** | #32 per-platform `auth.config` typing; **#23a–#23h** / `missingInRN` Other/Web exports; **#40** Other/Web `credentialFromResult` delegation |
173+
| **RN extensions (documented)** | #3, #4, #6, #9–#14, #30, #38, #39, `extraInRN` helpers |
174+
175+
---
176+
177+
## compare:types reason template
178+
179+
```text
180+
iOS/Android: [why native SDK won't match / permanent difference].
181+
182+
Other/Hermes: [not implemented | N/A | possible via js-sdk without DOM].
183+
184+
Other/Web: [not implemented | N/A | possible via js-sdk with DOM/browser APIs].
185+
186+
Other/All: [when both Other sub-contexts apply].
187+
188+
Omit paragraphs that don't apply.
189+
```
190+
191+
### Example (**#12** `disableWarnings`)
192+
193+
```text
194+
iOS/Android: disableWarnings is stored on auth.emulatorConfig only; native SDKs do not show a DOM emulator banner.
195+
196+
Other/Hermes: not applicable (no DOM).
197+
198+
Other/Web: not delegated yet; firebase-js-sdk disableWarnings DOM suppression is possible.
199+
```
200+
201+
---
202+
203+
## Quick reference
204+
205+
### Other/Web only (**W**)
206+
207+
**#9, #12, #13, #14, #15, #22, #23a, #23b, #23c, #23e, #23f**
208+
209+
### Other/All (**H+W**)
210+
211+
**#1, #2, #4, #16, #17, #18, #21, #23d, #23g** (+ **#28** implemented everywhere)
212+
213+
### Native / RN-only (no Other path)
214+
215+
**#3, #5, #7, #11, #39**
216+
217+
---
218+
219+
## Iteration log
220+
221+
| Round | Change |
222+
|-------|--------|
223+
| 1 | Won't change / Planned / Needs decisions (#1–#40) |
224+
| 2 | Unique IDs; **#23a–#23h** |
225+
| 3 | Other/Hermes vs Other/Web |
226+
| 4 | Dumped to this file |
227+
| 5 | **Update compare:types note** table; **#8** reclassified; **#28** implemented; **#24–#27** deprecated |
228+
| 6 | **#8/#37** closed (MFA on Other verified in `tests/local-tests`); **#29/#31** provider typing aligned; **#33** enumerable `additionalUserInfo` + `AdditionalUserInfoNative`; **#32** option B + **#36** document-only; registry + migration guide updated |
229+
| 7 | **#40** document-only; `credentialFromResult` future path on **Other/Web** via `RNFBAuthModule`; triage **#1–#40** complete |
230+
231+
**Future work (post–v25 typing):** Other/Web `credentialFromResult` delegation; optional per-platform `auth.config` typing; `missingInRN` browser/js-sdk exports per **#23a–#23h**.

0 commit comments

Comments
 (0)