Skip to content

fix: seed useUser with auth.currentUser to prevent undefined on initial render#731

Merged
tyler-reitz merged 7 commits into
FirebaseExtended:mainfrom
tyler-reitz:fix/use-user-initial-render
Jul 14, 2026
Merged

fix: seed useUser with auth.currentUser to prevent undefined on initial render#731
tyler-reitz merged 7 commits into
FirebaseExtended:mainfrom
tyler-reitz:fix/use-user-initial-render

Conversation

@tyler-reitz

@tyler-reitz tyler-reitz commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Since 4.2.3 (#539 removed the `initialData` seeding), `useUser()` returns `{status: 'loading', data: undefined}` on the first render when called in a child component whose parent only called `useSigninCheck()` as a guard.

The root cause: `useSigninCheck` and `useUser` use different observable IDs (`auth:signInCheck:...` vs `auth:user:...`). When the guard resolves and renders the child, the child's `useUser()` creates a fresh `SuspenseSubject` that hasn't received a value yet. Firebase's `onAuthStateChanged` fires asynchronously, so the first snapshot is always `loading`.

This breaks patterns like:

```tsx
function AuthGuard({ children, fallback }) {
const { status, data: signInCheckResult } = useSigninCheck();
if (status === 'loading') return fallback;
if (signInCheckResult.signedIn) return children;
return ;
}

function ProfilePage() {
const { data: user } = useUser(); // undefined on first render
const { data: idTokenResult } = useIdTokenResult(user as User, false); // throws
}
```

Fix

Restore the synchronous seed: if `auth.currentUser` is truthy (a signed-in user is available), set it as `initialData` before passing to `useObservable`. This means `useUser()` returns the user immediately on the first render without waiting for the async observable.

The check is `if (auth.currentUser)` (truthy, not `!== undefined`) to avoid the ambiguous case where `currentUser` is `null` before auth has finished loading from storage. If we seeded `null` in that case, callers would incorrectly see a signed-out state during initialization.

#539's objection was that `currentUser` was undocumented tri-state. That ground has shifted: the SDK now explicitly documents `currentUser: User | null` and exposes `authStateReady()`. Truthiness is still the right check here since `authStateReady()` is async and can't help a synchronous first render.

Testing

Added a regression test: `synchronously returns the current user without waiting for the observable`. The test installs `NEVER` as the `auth:user` observable before rendering, so any synchronous user value on first render must come from the `initialData` seeding path, not from the observable. Without the fix: `status: 'loading'`. With the fix: `status: 'success'`, `data: auth.currentUser`.

Fixes #582

Restores synchronous user access that was removed in 4.2.3 (FirebaseExtended#539).
Without it, child components calling useUser() see status:'loading' /
data:undefined on first render even when the user is already signed in,
because a fresh SuspenseSubject has to wait for the async
onAuthStateChanged callback before emitting.

Only seeds initialData when auth.currentUser is truthy, so the
uninitialized-auth case (currentUser is null before SDK reads from
storage) is not incorrectly treated as signed-out.

Fixes FirebaseExtended#582

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the useUser hook in src/auth.tsx to seed initialData with auth.currentUser when a user is already signed in, enabling synchronous rendering on the first load. The review feedback points out that this implementation unconditionally overwrites any caller-provided initialData or startWithValue options, and suggests only seeding the user if these options are not already explicitly defined.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/auth.tsx Outdated
tyler-reitz and others added 3 commits July 9, 2026 14:02

@armando-navarro armando-navarro left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two asks, non-blocking since I'm approving:

  1. Add a regression test that fails on main. The test you cite passes on main in test suite runs, so nothing today stops a future #539-style removal from silently regressing #582 again. The guard-pattern structure works even mid-suite because useSigninCheck and useUser cache under different ids: parent renders a useSigninCheck guard; once signedIn, mount a child calling useUser(); assert the child's first-render data is the user. Happy to share the exact test I used.
  2. Correct the Testing section of the PR body (it becomes the squash-commit message): "All 14 auth tests pass" is equally true on main, and the cited test only discriminates when run in isolation. With ask 1 done, the body can simply point at the new test.

A couple more non-blocking notes that didn't fit as inline comments:

  • Is it Intentional that useSigninCheck stays unseeded? Post-merge, the two auth hooks disagree on first-render state for a signed-in user (useUser → instant success, useSigninCheckloading flash). jhuleatt's comment in #582 pointed at the two hooks using different observables as the root problem; warming/sharing from useSigninCheck (no-claims path) would close the asymmetry and has no currentUser-timing windows at all. Fine as a follow-up — asking mainly whether it's on your radar.
  • Since #539's objection was "undocumented tri-state," worth one line in the body noting the ground has shifted: the SDK now documents currentUser: User | null and exposes authStateReady(). Truthiness is still a fine choice here (authStateReady() is async and can't help a synchronous first render), but pre-empting the #539 argument in the body will save a review cycle if Jeff looks at this.
  • A component that mounts while signOut() is in flight will now show the outgoing user on its first frame and see no loading status for that mount (success(user)success(null)). Main already showed a stale signed-in frame one paint later in the same window, so this is a one-frame delta, not a new failure mode — but if someone ever reports "flash of signed-in UI during logout," this is where it lives.

Approving.

Comment thread src/auth.tsx
// synchronously on the first render without waiting for the async observable.
// We only do this when currentUser is truthy to avoid masking the uninitialized
// (null before auth has loaded from storage) case as "signed out".
if (auth.currentUser && !('initialData' in _options) && !('startWithValue' in _options)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider documenting the behavior change where users will see it: one JSDoc sentence on useUser (first render is success for already-signed-in users; in suspense mode, signed-in users no longer suspend) + a docs regen. The suspense change is arguably the biggest surface change and it's currently only visible by reading this code comment.

Comment thread src/auth.tsx
// We only do this when currentUser is truthy to avoid masking the uninitialized
// (null before auth has loaded from storage) case as "signed out".
if (auth.currentUser && !('initialData' in _options) && !('startWithValue' in _options)) {
_options.initialData = auth.currentUser as unknown as T;

@armando-navarro armando-navarro Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialData's type collapses to any, so this assigns with no cast at all (verified with the repo's own tsc). If you keep a cast, the codebase's existing idiom is as any as.

@tyler-reitz

Copy link
Copy Markdown
Contributor Author

Added a regression test in 670733a that should address the ask.

The approach: before rendering, we replace the auth:user observable in the preloaded cache with NEVER (an observable that never emits). This means any synchronous user value on first render must come from the initialData seeding path (the fix), not from the observable. Without the fix the first render sees status:'loading'; with it, status:'success'.

One tradeoff worth noting: the test deletes the cache entry via (globalThis as any)._reactFirePreloadedObservables and hardcodes the internal key format auth:user:${auth.name}. The global is deliberately on globalThis for cross-module sharing, so it's stable in practice, but the cache key is an internal detail. If that format changes, the test would fail on main for the wrong reason rather than silently passing. Happy to extract a test utility (_deletePreloadedObservable) from useObservable.ts if you'd prefer to avoid the raw global access.

@tyler-reitz

Copy link
Copy Markdown
Contributor Author

Re: useSigninCheck staying unseeded — yes, it's on our radar. The asymmetry is real and the shared-observable approach you described (warm useSigninCheck's no-claims path, let useUser read from the same cache entry) would be the cleaner fix. Tracking it as a follow-up so it doesn't get lost.

@tyler-reitz

Copy link
Copy Markdown
Contributor Author

Code review also surfaced a pre-existing bug that this PR exercises more frequently: #736 (useObservable mutates the shared _immutableStatus snapshot in-place). Filed separately — not blocking merge.

The guard-pattern test passed on main because SuspenseSubject's warmup
subscription made onAuthStateChanged fire before getSnapshot() was called.

The new test installs NEVER as the auth:user observable so it can never emit,
isolating the synchronous-user guarantee to the initialData seeding path only.
Fails on main (status:'loading'), passes with the fix (status:'success').
@tyler-reitz
tyler-reitz force-pushed the fix/use-user-initial-render branch from 670733a to 52eacea Compare July 14, 2026 18:39
…ed snapshot

useObservable was mutating the object returned by getSnapshot() in-place.
That object is SuspenseSubject._immutableStatus, a single shared reference
across all components using the same observableId. One component's initialData
render would corrupt the snapshot for all other components.

Fixes FirebaseExtended#736
@tyler-reitz
tyler-reitz merged commit ebc87af into FirebaseExtended:main Jul 14, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

useUser() returns undefined unexpectedly on initial render

2 participants