FirebaseExtended / FirebaseExtended/reactfire

v5: remove deprecated and dead exports

オープン
#754 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
v5
主要言語
TypeScript
スター
3.6k
フォーク
403
平均マージ
14時間 53分
マージ済み PR(30日)
5

説明

### Background

reactfire ships several exports that are deprecated or dead but kept because removing them breaks consumers at load time, not just at compile time. v5 is the release that can drop them. **Scope widened 2026-08-03** from the two `checkOptions` helpers to every deprecated export, so the breaking-removals work is tracked in one place.

> **Corrected 2026-08-05.** An earlier revision of this issue described the work as a checklist of independent deletions. Call sites were checked against `v5` at `e7b18c2` and that is not accurate: one group is blocked on another PR, one is not dead code at all, and one requires test changes. The three groups below are sorted by what they actually cost, and **they are not independent**. Line references are as of `e7b18c2`.

---

### Group 1: `checkOptions` and `checkinitialData` (BLOCKED, do not start)

- [ ] Remove `checkOptions` from `src/index.ts` (`:34`)
- [ ] Remove `checkinitialData` from `src/index.ts` (`:43`)

⚠️ **These are not dead on `v5` today.** `checkIdField` (`src/index.ts:47`) still calls `checkOptions`, and so does `checkinitialData`. The earlier claim that they became unused "as of #740" is true of #740's branch, which rewrites `checkIdField` to read `options?.idField` directly, but **#740 targets `main` and has not merged**, so `v5` has not received that rewrite.

Doing this group now would mean duplicating #740's change on `v5`, which then conflicts when `main` is forward-integrated. **Blocked until #740 merges and is forward-integrated.** `checkIdField` itself stays either way; it is still used by the Firestore and Database data hooks.

### Group 2: `startWithValue` (NOT a deletion, it is a behavior change)

- [ ] Remove `startWithValue` from `ReactFireOptions` (`src/index.ts:30`), marked `@deprecated use initialData instead`

⚠️ **This is not dead code.** It has three live call sites on `v5`:

- `src/useObservable.ts` (`:113`): `config?.initialData ?? config?.startWithValue`, the actual fallback that makes the option work
- `src/useObservable.ts` (`:78`): the `hasInitialData` check that decides whether to skip `loading`
- `src/auth.tsx` (`:34`): `useUser` will not seed `initialData` from `auth.currentUser` if the caller passed `startWithValue`

So removing it changes runtime behavior for anyone still passing it, rather than only failing their typecheck. It needs the fallback logic removed alongside the field, and an upgrade-guide entry pointing at `initialData`. Independent of Group 1.

### Group 3: `ClaimsCheck` and `AuthCheck` (self-contained, but touches tests)

- [ ] Remove `ClaimsCheck` (`src/auth.tsx:215`) and its exported `ClaimsCheckProps` (`:60`)
- [ ] Remove `AuthCheck` (`src/auth.tsx:259`) and its exported `AuthCheckProps` (`:54`)

Both are `@deprecated Use useSigninCheck instead`, both only function in experimental Suspense mode and `console.warn` otherwise. **They must go together**: `AuthCheck` renders `ClaimsCheck` internally (`src/auth.tsx:271`).

⚠️ **`test/auth.test.tsx` needs more than deletion.** It defines an `AuthCheckWrapper` and reuses it at `:285-300` and `:369` inside a `useUser` test that is not about `AuthCheck` at all. That test needs rewriting to use a plain provider, not removing.

**Do not also remove `ClaimCheckErrors` (`:67`)**, despite it sitting between the two interfaces. It is part of the `SigninCheckResult` shape (`:81`, `:96`) and stays.

### Then, once the groups above land

- [ ] Regenerate reference docs (`npm run docs:fork`, not `npm run docs`) to drop the corresponding pages
- [ ] Record each removal in the v5 upgrade guide with its replacement (`initialData` for `startWithValue`, `useSigninCheck` for both components)

### Notes

- Each of these is a **runtime** break for plain JS importers (an ESM import error at load), not only a type error. That is why they were deferred rather than done in a patch.
- After #740's tightening, `checkinitialData`'s inferred return type becomes `unknown` (was `any`). Harmless while unused, and another reason to retire it.
- **Deliberately not folded into #740.** That PR tightens `ReactFireOptions` generics, its squash body becomes the changelog, and a removal buried under a `fix:` title is how a break gets missed. Same release, separate PR.
- **Groups 2 and 3 can proceed while Group 1 is blocked.** Splitting this into more than one PR is reasonable; a single PR mixing a behavior change with two component removals makes the changelog harder to read.

Context: #740, #741.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

Start by reading the v5 call sites in src/index.ts, src/useObservable.ts, and src/auth.tsx, and check #740 before touching the blocked Group 1. For Group 3, inspect test/auth.test.tsx and its AuthCheckWrapper usage; completion also requires npm run docs:fork and v5 upgrade-guide entries for the replacements.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
firebase, react, typescript
領域
authentication, documentation, frontend
issue の種類
リファクタリング
難易度
5/5
見積もり時間
1週間以上
活発さ
静か
明瞭さ
明確に書かれている
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。