Skip to content

demo(recipe): swap the hand-rolled hooks for ReactFire - #798

Open
tyler-reitz wants to merge 3 commits into
recipe-demofrom
recipe-demo-framework
Open

demo(recipe): swap the hand-rolled hooks for ReactFire#798
tyler-reitz wants to merge 3 commits into
recipe-demofrom
recipe-demo-framework

Conversation

@tyler-reitz

@tyler-reitz tyler-reitz commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Swaps the vanilla app's hand-rolled Firestore and auth plumbing for ReactFire. The diff against recipe-demo is the deliverable: same app, same behavior, nothing added.

Not a merge candidate. This branch and #797 exist to be compared and read, not landed.

What ReactFire replaces

Deleted Replaced by
src/lib/use-recipes.ts, 44 lines of hydrate-then-subscribe useFirestoreCollectionData(query, { idField, initialData })
src/lib/session-context.tsx, 18 lines of context over onAuthStateChanged useUser() and useSigninCheck(), called directly

Nine files under src/ plus package.json, +42 / -94. The base branch's commit message predicts five files; that assumed keeping both modules as thin wrappers over ReactFire, which was rejected because it preserves the abstraction ReactFire is meant to obviate and hides useSigninCheck entirely. src/lib/providers.tsx (16 lines) is new and wires the three providers to the app's existing module-level instances. recipes.ts, types.ts, ai.ts, firebase.ts, RecipeList.tsx and all three pages are untouched.

The homepage still fetches on the server with the Firebase JS SDK, not the Admin SDK, and hands the result to a client component as initialData. The recipe list is public, so no identity is needed and the security rules stay in play.

Findings

This app does not build against published reactfire. SessionNav calls useUser from the root layout, so every prerendered route hits a reactfire hook:

Error occurred prerendering page "/_not-found"
Error: Missing getServerSnapshot, which is required for server-rendered content.
    at i (.next/server/app/signin/page.js:1:8950)

Measured both ways: patched build → 8 <article> in the server HTML; published 4.2.6 → build fails; patched build restored → 8 again. #779 is therefore a hard prerequisite for this demo, not an improvement to it.

Error handling is where reactfire is worse. RecipeBrowser had an error branch rendering the failure inline. useObservable on main re-throws unconditionally, suspense: false or not, so that UI cannot work and the branch is deleted rather than left dead. The vanilla hook kept the page up and showed the error; this version unmounts to the nearest boundary. #735 fixes exactly this and is v5-only, so it is the obvious candidate for a round two.

A cast is unavoidable. useFirestoreCollectionData<Recipe> requires a Query<Recipe>, but recipeQuery returns Query<DocumentData>, so the generic moves the error rather than solving it. Typing it properly means a withConverter in recipes.ts, which would put Firestore types back into the file the swap is meant to leave alone. data as Recipe[] at the call site is the smaller cost.

Hydrated versus live is off the status enum. The distinction the Firebase SSR blog post's hand-rolled useSerializedQuery tracks is not something reactfire's status can carry. Measured against the emulator with initialData passed: while the server data is still on screen, status is already success and hasEmitted is already true, and only firstValuePromise resolving marks the first live snapshot. So the capability exists, but only by awaiting a promise and mirroring it into local state, which is the bookkeeping the swap is meant to remove. The vanilla branch previously declared four feed states while rendering two; both sides now carry the same three, and this is the reason why.

Two pages are byte-identical to vanilla, which is also a result: signin/page.tsx, since there is no sign-in wrapper, and create-recipe/page.tsx, since there is no AI Logic binding and the page calls the SDK directly. RecipeCard changes only its user lookup; the like itself is plain updateDoc either way. RequireAuth barely shrinks, because useSigninCheck still returns a status and the loading-then-redirect dance survives.

The dependency

reactfire is a packed build of main at ac3ccf9, committed under recipe-demo/ with a .gitignore exception. Regenerate with npx tsc && npx vite build && npm pack at that commit. A file:.. link was rejected because npm symlinks the repo root in, bringing a second React with it.

Verification

Against the emulator suite, controls run in both directions:

Check Result
Server render 8 <article> with <script> blocks stripped, negative control on an absent title
Cuisine filter Japanese returns exactly 2
Like Like (0)Liked (1), page sentinel survived so no reload
Live subscription An external write from a node script appeared as Like (1) with no interaction, and rendered as "Like" rather than "Liked" because a different uid made it
Route protection /create-recipe signed out redirects to /signin?next=%2Fcreate-recipe
Console No errors and no hydration warnings; the reader was confirmed able to report by planting one
tsc Clean, confirmed able to fail first

Not verified: /create-recipe generation, which needs a real project with AI Logic enabled. Same gap as the vanilla half.

AGENTS.md and CLAUDE.md in this folder are written by next dev when it detects an AI coding agent, never by next build, and agentRules: false turns the behaviour off. They are Next's own instructions to coding agents, not ours, and are committed so the tree stays clean.

Replaces the app's own Firestore and auth plumbing with ReactFire hooks,
so the diff against recipe-demo is the comparison itself.

- src/lib/use-recipes.ts (42 lines) becomes useFirestoreCollectionData
  with initialData, keeping the server-fetched list as the seed
- src/lib/session-context.tsx (17 lines) is deleted; useUser and
  useSigninCheck are called from the components that need them
- src/lib/providers.tsx wires FirebaseAppProvider, FirestoreProvider and
  AuthProvider to the existing module-level instances

The dependency is a packed build of main at ac3ccf9, committed under
recipe-demo/ with a gitignore exception. Published 4.2.6 predates #779,
and without it this app does not build: SessionNav calls useUser from the
root layout, so prerendering /_not-found and /signin fails with "Missing
getServerSnapshot". Measured both ways, with the patched build restored
afterwards.

RecipeBrowser loses its error branch because useObservable on main
re-throws rather than returning status 'error', and the recipe list needs
a cast because useFirestoreCollectionData's generic requires a typed
Query. Both are findings for the PR description rather than problems to
work around here.
# Conflicts:
#	recipe-demo/src/lib/use-recipes.ts

@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.

What I want to raise is a theme rather than a list of defects: the comparison currently undersells what adopting ReactFire costs here. You disclose the error-handling regression, and I found four more behaviours that changed without a mention. None of it blocks anything, since the branch is not going anywhere, but each is something a reader of the diff would want to know.

What the ReactFire side loses, that the diff does not show

The cuisine filter shows the wrong recipes

initialData: initialRecipes is the server's unfiltered list, and it is handed to every filter rather than only the one it belongs to.

  • useObservable's overlay forces status: 'success' for it, so selecting a cuisine paints the whole unfiltered list, labelled as that cuisine's result, until the real snapshot lands.
  • loading={status === 'loading'} can therefore never be true, so RecipeList's aria-busy branch is unreachable here and live on the vanilla side.
  • The guard against this is in the file the PR deletes. use-recipes.ts at d4d1af1, the swap commit's own parent, carries renderedCuisine and if (cuisine !== renderedCuisine.current) setStatus('loading').
  • What worked when I tried it: omit the key on other filters, cuisine === 'all' ? { idField: 'id', initialData: initialRecipes } : { idField: 'id' }. Four lines for four, so your +42 / -94 is unchanged.
  • The shorter initialData: cuisine === 'all' ? initialRecipes : undefined does not work, which I found by trying it first. useObservable tests hasOwnProperty('initialData'), so the key being present is what counts, not its value.
  • That form therefore reports success with data undefined, which is a TypeError in RecipeList and typechecks clean.

createdAt changes type under the same declared type

Recipe.createdAt is declared string, and toRecipes used to normalise it on every snapshot.

  • rxfire returns raw document data, so the seeded render carries a string and live snapshots carry a Firestore Timestamp. I saw both in one session.
  • data as Recipe[] asserts that away, and nothing renders the field today, which is what makes it easy to inherit.
  • The first new Date(recipe.createdAt) a reader writes behaves differently depending on which render it lands in.
  • Your withConverter aside is the fix. I would frame it as a correctness fix rather than a typing nicety.

Filter subscriptions are never released

Each cuisine opens a listener that outlives the switch away from it.

  • I subscribed to Japanese, switched to Mexican, then wrote a Japanese recipe while nobody was viewing that filter.
  • The abandoned subject went from two documents to three, so it was still live and still receiving server updates.
  • The vanilla hook returned onSnapshot's unsubscribe from its effect, so every filter change tore the previous listener down.

There is an ordering where hydration mismatches

I could only produce it by forcing that order, so treat this as a hazard rather than something I saw happen on its own. useUser seeds from auth.currentUser only when it is truthy, and on the server that is always null.

  • The server HTML is <li aria-busy="true"><span>Loading</span></li>.
  • If the browser's session restores before hydration, the client takes the seeded branch and renders the email instead.
  • Forcing that order gave me React's "Hydration failed because the server rendered text didn't match the client", settling on the email.
  • The deleted SessionProvider could not do this. It always started at { user: null, status: 'loading' } and only moved inside an effect, which never runs before hydration.
  • I could not show the race happens on its own. Auth restores persistence from IndexedDB asynchronously, so currentUser is normally still null when React hydrates.
  • Your Console row is itself evidence it does not fire in ordinary use, and I am not disputing that row.
  • Raising it because SessionNav is in the root layout, so if it does fire it is every route, and RecipeCard's disabled={!user} is exposed the same way.

After the first mount, the server payload is fetched and thrown away

page.tsx is force-dynamic, so every soft navigation back to / re-runs the fetch and ships a fresh initialRecipes.

  • The cached observable already has a value, so useObservable skips the overlay and never reads the payload.
  • I checked with a sentinel payload and it never appeared on the return visit.
  • The deleted hook used useState(initialRecipes), so it painted the server list on every remount.

On the error section

Your re-throw reasoning is right, and deleting the branch rather than leaving it dead was the correct call. One sentence I would change, and one thing I would not do.

  • "Unmounts to the nearest boundary" understates it. There is no route-level error.tsx, so the throw escapes past the root layout to Next's built-in top-level boundary, which replaces the whole document including the nav.
  • The vanilla branch kept the page up and named the failure inline, which is the comparison worth stating.
  • I would not add an error.tsx to fix it, because that puts code on one side only and moves the diff the deliverable rests on.
  • On my reading of how Next nests the segment boundary it would not catch SessionNav anyway, since the root layout renders that above the route's own boundary.
  • Worth pairing with it: the cuisine filter needs a composite index that neither branch ships, so on a real project the first cuisine selection is the likely trigger, and it now takes the page rather than rendering inline.

The committed dependency

This repo publishes an exp-tagged build per commit to main, and 4.2.6-exp.ac3ccf9 is on npm from exactly the commit you packed. Its src/ is byte-identical to the tarball's and the demo builds against it, so the binary and its supporting machinery could all go.

  • Pin it exactly. npm i reactfire@4.2.6-exp.ac3ccf9 writes ^4.2.6-exp.ac3ccf9, and that caret matches the published 4.2.6, which is the release you correctly say cannot build this app.
  • A clean resolve picks 4.2.6, which has no getServerSnapshot at all, so the demo lands back on the failure your Findings section leads with.
  • "reactfire": "4.2.6-exp.ac3ccf9" with no caret resolves correctly.
  • The regeneration recipe does not reproduce the file. package.json at ac3ccf9 is version 4.2.6, so npm pack emits reactfire-4.2.6.tgz, and the rename is undocumented.
  • The vendored copy declares itself 4.2.6, so npm ls reactfire cannot tell it from the published release.
  • Its dist/index.js also carries build-path comments from the machine that packed it (//#region ../../../../../../home/me/reactfire/...), which the npm build does not. Nothing runtime, but it means the bytes cannot be reproduced from this repo.

Smaller

  • <FirestoreProvider sdk={firestore}> has no consumer here: useFirestoreCollectionData takes its instance from the query it is handed, and nothing calls useFirestore().

  • I removed the provider and its imports and the app still compiled, so a reader counting three providers is counting one that does nothing here.

  • The swap changes which auth listener runs. rxfire's user() wraps onIdTokenChanged where authState() wraps onAuthStateChanged, so components re-render on token refresh rather than only on sign-in and sign-out.

  • useUser and useSigninCheck register under different observable ids, so /create-recipe holds two auth subscriptions where the deleted context held one and fanned it out.

  • RequireAuth grew rather than shrank: 8 added, 7 deleted, 29 lines to 30. Small, but it sits in the paragraph making the line-count argument, where everything around it is exact.

If I have misread any of these, particularly the hydration ordering or the filter behaviour, tell me which and I will run it again.

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.

2 participants