Skip to content

demo(ai-studio): swap the live queries for ReactFire - #800

Open
tyler-reitz wants to merge 5 commits into
ai-studio-demofrom
ai-studio-demo-framework
Open

demo(ai-studio): swap the live queries for ReactFire#800
tyler-reitz wants to merge 5 commits into
ai-studio-demofrom
ai-studio-demo-framework

Conversation

@tyler-reitz

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

Copy link
Copy Markdown
Contributor

Swaps the AI Studio export's two live Firestore subscriptions for ReactFire hooks. The diff against ai-studio-demo is the deliverable: same app, same behavior, nothing added, the 1426-line App.tsx kept as one file per the requirements doc.

Not a merge candidate. Comparison artifact, like its base.

What converted and what did not

Site What it is Converted?
onSnapshot households live household list for the signed-in user YesHouseholdsFeed
onSnapshot recipes live recipes for the selected household YesRecipesFeed
onAuthStateChanged on-sign-in lifecycle: profile creation, 24h cleanup No, by decision: it is a routine, not a state binding. useUser and useSigninCheck therefore never appear in this demo; project 1 covers both
getDocs ×4 imperative deletes (cleanup routine, cascade delete) No: event-handler mutations are outside a data-binding library's remit, same as toggleLike in project 1. Zero diff is the honest output

The headline: hooks cannot be conditional, and one mount point is not enough

Both original effects bail out early (if (!user) return, if (!user || !selectedHousehold) return). A hook cannot opt out of running, so each subscription became a child component mounted behind its guard, pointing at #346 (disabling queries, 8 reactions) and #463 (nullable refs, 18 reactions).

It is worse than one extraction per subscription: App renders four mutually exclusive screens, and the feeds must be mounted in every branch a signed-in user can reach, three of the four. Both incomplete configurations were built and driven:

  • Feeds only in the main return: the app comes to rest on the spinner, because householdsLoading starts true and only the feed's callback clears it.
  • Feeds in the spinner branch but not onboarding: this is the true deadlock. households is empty, onboarding renders, the feed never mounts, and nothing can populate households. Submitting the form leaves the screen on onboarding.

An earlier version of this section attributed the deadlock to the first configuration. Corrected by Armando Navarro, who built both. The conclusion is unchanged: all three mount points are required, and the vanilla effect ran regardless of which screen rendered. Rebuilding that property by hand is part of the swap's cost.

The measurement

+106 / -42 across 3 files (App.tsx +96/-41, main.tsx +9/-1, package.json +1), measured from the repoint commit so the config changes are not counted against the swap. That range also carries the lockfile, and it excludes package.json's lockfile churn, so read it as the cost in code rather than the total cost of adopting the dependency. Eight of the main.tsx lines are provider ceremony that this app does not need, see below. The net +64 lines was the predicted outcome, stated in the spec up front: for an app shaped like this one, ReactFire pushes generated code toward more structure, not less. On the token question the doc actually asks: extrapolating from this diff, a generator writing this app with ReactFire available would have spent more tokens on the data layer, not fewer. That is an estimate derived from the line delta, not a generation measurement.

Other findings

  • 🔴 Signing out and back in blanks the app, and only a reload recovers. Under the export's own firestore.rules: sign in, sign out, sign back in as the same account, and the React root empties and is still empty six seconds later. The vanilla half signs straight back in. Same for user A signing out and user B signing in on the same tab. Cause: sign-out gives both cached entries permission-denied, the cache never evicts or retries (The observable cache leaks subscriptions and is shared across SSR requests #790, Error recovery: no retry path once an observable cache entry errors #742), so the same query maps back to the same poisoned entry and useObservable rethrows during render. Control: clearing _reactFirePreloadedObservables immediately before signing back in, changing nothing else, recovers completely. The vanilla half is not handling the error better, it is never in a position to receive one, because its effect cleanup unsubscribes when user changes. Found by Armando Navarro.
  • 🔴 A user with no households can get a spinner that never ends. Production build only (vite build plus vite preview), permissive rules, on the second sign-in for the same user. Still spinning at 33 seconds, with the cache entry reading success and zero households, so the data arrived and the screen never advanced. The dev server recovers, which is why the browser checks missed it. Behaviour reproduced, cause not established. Also Armando's.
  • ⚠️ The rules caveat in an earlier version of this body was backwards. It said the export's own rules are unexercised "on both halves equally". Switching the real rules on is what exposes the blank page above, so permissive rules mask a ReactFire-only failure rather than leaving both halves equally untested. Permissive rules are not a clean bill of health either: the spinner hang happens under them.
  • The three providers do nothing in this app. main.tsx was reverted byte for byte to its pre-swap content and both feeds still render live, because useFirestoreCollectionData takes its instance from the query it is handed and nothing reads a context except the suspense flag. AuthProvider and FirestoreProvider can each be dropped, though neither can be kept without FirebaseAppProvider above it. They are kept here because they are what the documentation tells you to write, and the eight lines of ceremony are part of the honest cost.
  • The two arms run different listener configurations. rxfire's collectionData subscribes with includeMetadataChanges: true, against its own default of false, where the onSnapshot calls it replaced passed a callback first and so took the SDK default. Nothing renders wrong, but it matters for a comparison that is partly about render cost.
  • The error path is lost. useObservable re-throws unconditionally, so both handleFirestoreError callbacks are gone: Firestore errors now reach the app's ErrorBoundary from the main screen, and nothing at all from the spinner and onboarding screens. Same finding as project 1; fix: surface observable errors via status instead of re-throwing #735 (v5-only) fixes exactly this.
  • One delta favours ReactFire and is a side effect of a known leak, not a feature: previously viewed households stay subscribed because the observable cache never evicts (The observable cache leaks subscriptions and is shared across SSR requests #790), so switching back renders instantly and stays synced while unselected, where vanilla tears the listener down. Disclosed here so a snappier-feeling demo is not read as a capability.
  • Unchanged from vanilla, same class as the demo(recipe): vanilla recipe app on the Firebase JS SDK #797 review's finding 2: one recipe document missing an array field (instructions) blanks the whole app. Reproduced accidentally during verification with a malformed probe document; both halves crash identically.
  • The sort moves out of the snapshot callback and must operate on a copy, since sorting ReactFire's data in place would mutate its cached value.

Verification

Against the Firebase emulators, in the browser, each check paired with a control:

Check Result
Existing user renders households and recipes Same as vanilla
Onboarding exit (the deadlock case) Fresh uid creates a household through the UI and leaves onboarding, no reload
Live: external rename of the selected household Header updates, no reload
Live: external recipe write Appears sorted first, no reload
Sign out and sign back in ⚠️ Added after review, and it fails on this half. Blank root under the export's own rules; the vanilla half recovers. See findings
Sort createdAt descending across probe and seeded recipes
Distinguishability No-household (onboarding) vs empty household (empty grid) remain distinct states, which the rejected sentinel-query design would have merged
Mutation controls idField broken → blank page with a where() error; recipes handler neutered → empty grid. Both restored and re-verified
tsc and vite build Clean, typecheck first proven able to fail

Unverified: signInWithPopup (needs a focused window and a human; sessions were established via signInWithCredential against the Auth emulator), Gemini generation (needs the AI Studio key), and the export's own firestore.rules: every check ran under the repo's open emulator rules, so the app's real security model is unexercised on both halves equally.

…itch

The export hard-codes the Google-internal project makersuite-showcase with
a named Firestore database, so it cannot run anywhere else. Repoints the
config and adds VITE_USE_EMULATORS so the same build serves local
verification and a real project later. The named database id is kept:
the emulator serves named databases (verified 2026-08-20 by write and
isolation, since a read probe returns 200 from any database name).

tsconfig gains a types array because the export never referenced
vite/client, so import.meta.env did not typecheck; the other three
entries keep the ambient types the no-types default was already loading.

Verified against the emulators: console sign-in with an unsigned Google
credential lands on onboarding, and the negative control (emulator down)
surfaces testConnection's offline error in the console, which stops on
restart. The UI is not the signal for that control; the sign-in screen
renders normally either way.

Separate from the ReactFire swap so that diff shows only the swap.
Providers first with nothing consuming them, so any breakage here is
attributable before a conversion is layered on top. The dependency is the
same pinned build recipe-demo uses (main at ac3ccf9), extracted from git
with matching hashes, and the gitignore negation is proven both ways: the
demo tarball is visible to git while a root-level one stays ignored.

Verified in the browser against the emulators: sign-in, household
creation, onboarding exit and the seeded recipe grid all behave exactly
as the unwrapped app, with no console errors. npm ls react shows one
react@19.2.4 with every consumer deduped to it.
The hook cannot be called conditionally and the query needs a signed-in
user, so the subscription lives in HouseholdsFeed, mounted behind a user
guard. One mount point is not enough: App renders four mutually exclusive
screens, and the feed must be mounted in every one a signed-in user can
reach (spinner, onboarding, main), or the app deadlocks on onboarding
with the subscription unmounted and households stuck empty.

The selection logic moves into a useCallback handler unchanged. The
!user bail-out and the loading-true-on-user-change behavior are
reproduced exactly, including leaving a stale household list on sign-out
as the original did.

The onSnapshot error callback is gone: useObservable re-throws rather
than surfacing an error status, so Firestore errors reach the app's
ErrorBoundary (main screen) or nothing (spinner and onboarding screens)
instead of handleFirestoreError.

Verified against the emulators in the browser: existing user renders,
fresh user exits onboarding by creating a household through the UI (the
deadlock case), an external rename of the selected household streams into
the header with no reload, and the idField mutation control blanks the
page with a where() error, proving the feed is what drives the list.
Both subscriptions now come from ReactFire. Neither hook can be called
conditionally, so each lives in a child component mounted only when its
precondition holds, in every return branch a signed-in user can reach:
the surrounding selection logic and sort are unchanged and merely moved
out of the snapshot callbacks. The sort operates on a copy, since sorting
ReactFire's data in place would mutate its cached value. The now-unused
onSnapshot import is dropped.

The onSnapshot error callbacks are gone. useObservable re-throws rather
than surfacing an error status, so Firestore errors now reach the app's
ErrorBoundary instead of handleFirestoreError.

Verified against the emulators in the browser: switching households
switches recipe lists, an external write appears sorted first with no
reload, createdAt-descending order holds across probe and seeded
recipes, no-household-selected (onboarding) stays distinguishable from
household-with-no-recipes (empty grid), and the mutation control
(dropping the handler's data) empties the grid, restored and re-verified
after.

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

The app picks up two failures in this swap that the comparison does not currently show, and each one hides from a different part of how you tested.

A signed-out user who signs back in gets a blank page

Under the export's own firestore.rules, on the ReactFire half:

  • Sign in, sign out, then sign back in as the same account, and #root empties and is still empty six seconds later. Only a full page reload recovers.
  • The vanilla half, same emulator and same rules and same sequence, signs straight back in with no reload.
  • On sign-out both cached entries take permission-denied from the rules, and because the observable cache never evicts and never retries (#790, #742), the same query maps back to the same poisoned entry when you return. useObservable on main then rethrows during render (#735 fixes that shape, but only on v5).
  • The control that pins the cause: clearing _reactFirePreloadedObservables immediately before signing back in, changing nothing else, makes it recover completely.

A variant that needs nobody to sign back in as themselves, user A signs out and user B signs in on the same tab:

  • The ReactFire half goes blank there too.
  • The reason is that sign-out clears neither households nor selectedHousehold, so the recipes feed remounts against A's poisoned entry.
  • The vanilla half puts B on the onboarding screen with nothing of A's on it.

The vanilla half is not handling the error better, it is never in a position to receive one:

  • Its effect cleanup unsubscribes when user changes, so the listener is gone before the rules can reject anything, and nothing reached it after sign-out when I checked.
  • ReactFire's cached observable outlives the component that mounted it. With permissive rules I watched a recipe written after sign-out still arrive in the cached entry, so the underlying listener is still open.
  • That is what turns a sign-out into a permanent error once the rules start rejecting it.

If you want the failure to at least be recoverable, wrap the spinner and onboarding mount sites in ErrorBoundary instead of a bare fragment:

  • The same sequence then ends on the app's own "Something went wrong / Refresh App" screen rather than a white one.
  • I ran tsc --noEmit and vite build on it and drove the sequence in the browser.
  • It does not fix the poisoning, it only contains it, so the user still has to reload.

A user with no households gets a spinner that never stops, in the setup you tested

This one happens under the permissive rules your checks ran in, and only in a production build:

  • vite build plus vite preview, permissive rules, a user with no households. First sign-in reaches the onboarding screen correctly.
  • Sign out and sign back in as that same user, and the app sits on the spinner. It was still spinning at about 33 seconds.
  • Nothing errored. The cache entry reads success with zero households, so the data arrived and the screen never advanced.
  • The same sequence on the dev server recovers correctly, which I think is why the browser checks did not catch it.
  • A user who already has a household is unaffected in the same build.

I have the behaviour nailed down but not the explanation. The reading I find most plausible is that the feed's mount effect clears householdsLoading before App's own [user] effect sets it back to true, and the fragment root lets React reconcile the feed in place so no fresh emission ever arrives to clear it again. Your verification table has no sign-out and re-sign-in row, and I think it wants one on both halves.

The rules caveat points the other way round

The body says the export's own rules are unexercised "on both halves equally". From what I measured that undersells it:

  • Switching the real rules on is what exposes the blank page.
  • With permissive rules that particular failure disappears entirely, so the configuration masks it rather than leaving both halves equally untested.
  • Permissive rules are not a clean bill of health either, since the spinner hang above happens under them.

The three providers do nothing in this demo

I reverted main.tsx byte for byte to its pre-swap content and the app still renders households and recipes with both feeds live. useFirestoreCollectionData takes the query you built from the module-level db and never reads a context except the suspense flag, which defaults to false with no provider.

On what is actually removable:

  • AuthProvider and FirestoreProvider can each be dropped on their own.
  • What you cannot do is keep either one without FirebaseAppProvider above it, since both call useFirebaseApp() internally.
  • Eight of the lines in the measured swap cost are ceremony, the nine main.tsx adds minus the <App /> line it re-indents.

Given the number is the deliverable, I would either drop them, or keep them and say in the text that they are what the docs tell you to write rather than what this app needs.

The memoization comment sits on the handler that does not need it

I put a counter in each feed's effect and ran three configurations against the same data:

  • As written, both memoized: both counters settle in single digits and stop.
  • handleHouseholds without useCallback: still settles in single digits. No loop.
  • handleRecipes without useCallback: thousands of runs within five seconds and still climbing at eleven.

So the loop warning is attached to handleHouseholds, which does not loop, while handleRecipes, which does, carries no note. The difference is that handleRecipes allocates a fresh sorted array every call, so setRecipes re-renders every time. In a diff meant to be read and learned from, I think that comment wants to move, and to name the allocation as the reason.

The deadlock paragraph describes the other configuration

The headline section says that mounted only in the main return, the app deadlocks on onboarding. I built both configurations:

  • Feeds only in the main return: the app comes to rest on the spinner, because householdsLoading starts true and only the feed's callback clears it. Onboarding is not where it lands: on a fresh load with a restored session I never caught it rendering at all.
  • Feeds kept in the spinner branch and dropped only from onboarding: this is the one that rests on onboarding, and it is a real deadlock. I submitted the form and the screen was still on onboarding eight seconds later.

The mechanism you describe is right, it is just attached to the configuration that hangs earlier. Your conclusion that all three mount points are needed holds either way, and the onboarding-exit check passes on the PR as written.

Two casts that do not need the unknown hop

Three things I checked with the repo's own typecheck:

  • data as unknown as Household[] and data as unknown as Recipe[] both compile as a plain as Household[] and as Recipe[].
  • Routing through unknown switches off a check the single cast keeps. With the target swapped to something incompatible, the single-cast form additionally reports TS2352 ("neither type sufficiently overlaps") where the unknown form stays silent about the conversion itself.
  • Casting the collection reference instead (collection(db, 'households') as CollectionReference<Household>) also compiles and takes the cast off the data path entirely, if you prefer the types to read forward.

The generic form useFirestoreCollectionData<Household>(...) does not compile, because the query is a Query<DocumentData, DocumentData>.

Smaller things

  • rxfire's collectionData subscribes with includeMetadataChanges: true (collectionData calls collection(query), which calls fromRef(query, { includeMetadataChanges: true }), against rxfire's own default of false), where the onSnapshot calls it replaced passed a callback as the first argument and so kept the SDK's default options literal, { includeMetadataChanges: false }. Nothing renders wrong, but the two arms are running different listener configurations, which matters for a diff that is partly about render cost.

  • Following the committed instructions gets you a running dev server and a sign-in screen, and nothing past that: Firestore reports the named database as not found, and clicking sign in fails on the placeholder API key. VITE_USE_EMULATORS is only ever set in .env.example, which Vite does not load, and README.md is untouched and still describes the three-step AI Studio flow. Since the export could not run locally at all before your repoint commit this is a gap in new instructions rather than a regression, but the reproduction steps are part of the deliverable here.

  • {user && ...} in the onboarding branch and the main return can never be false, since if (!user) return sits above both. Only the spinner branch needs it.

  • h and hh in handleHouseholds land on added lines, though both names come verbatim off the lines the swap deletes, so the asymmetry against the sibling handler's fetchedRecipes is the export's rather than yours. Since these are now parameters on a component-boundary callback rather than locals inside a snapshot callback, they are worth renaming while they are being touched. It takes two renames, not one.

  • The "types" array in tsconfig.json only needs vite/client. The other three entries, node, react and express, all compile away. Related: it names react, but @types/react is not in package.json and arrives only as a transitive peer of react-markdown.

  • "+106 / -42 across 3 files" is exactly right for those three files. The same range also carries package-lock.json and the 131,774-byte tarball, and it counts package.json +1 without its lockfile, so a reader taking that as the total cost of adopting the dependency is missing a bit.

The first two are cases where someone reading the diff would conclude the swap is behaviour-preserving when it is not, so those are the two I would most want reflected in the text. If you read the intent of the comparison differently, say so and I will take another look.

The vendored tarball is unnecessary: 4.2.6-exp.ac3ccf9 is published from the
same commit. Pinned exactly, since the caret npm writes resolves to published
4.2.6 on a clean install.

The useCallback loop warning was on handleHouseholds, which does not loop.
Moved to handleRecipes, which does, and named the reason: it allocates a fresh
sorted array every call.

Dropped the unknown hop from both casts. It is not needed to compile and it
suppresses TS2352, so the single cast is strictly safer.

tsconfig types only needs vite/client. README run steps now say to copy
.env.example to .env.local, which is the only file Vite loads.

Found by Armando Navarro in review of #800.
@tyler-reitz

Copy link
Copy Markdown
Contributor Author

Both failures are real and both are now in the body. The first one changed how I think about #790, so I have escalated it there separately rather than leaving it as a demo note.

On the blank page: your control is what makes it, and I could not have argued past it. Clearing _reactFirePreloadedObservables and changing nothing else recovering the app pins the cause to the cache rather than to the error path. The point that the vanilla half is never in a position to receive the error, rather than handling it better, is the part I had wrong in my head, and it is now stated that way. I have written #790 up as an availability bug rather than a leak: the framing everywhere, including my own notes, assumed a browser session bounded the damage, and it does not. It lands inside one session on the first sign-out.

The spinner one I am recording as behaviour reproduced, cause not established, in your words rather than mine, since you have it pinned to a production build and I cannot improve on the explanation. The verification table now has the sign-out and back-in row on both halves.

Taken your other corrections as written:

  • The deadlock paragraph described the wrong configuration. Feeds only in the main return rests on the spinner; the true deadlock is feeds in the spinner branch but not onboarding. Both are now described, and the conclusion is unchanged.
  • The rules caveat was backwards and now says so: real rules expose the blank page, permissive rules mask it.
  • Providers kept, with the text now saying they are documentation ceremony this app does not need, and that eight of the measured lines are that. Dropping them would make the diff smaller and less honest.
  • The memoization comment has moved to handleRecipes, naming the fresh-array allocation as the reason. Your three-configuration counter measurement is what settled it.
  • Both casts lost the unknown hop. I checked your TS2352 point: with the target swapped to something incompatible the single cast reports it and the unknown form stays silent, so the single cast is strictly safer.
  • tsconfig types down to vite/client, README run steps now say to copy .env.example to .env.local, which is the only file Vite loads.
  • The tarball is gone: 4.2.6-exp.ac3ccf9 is published from the same commit, pinned exactly since the caret npm writes resolves to published 4.2.6.

Left as they are: the {user && ...} checks and the h/hh names both come verbatim off deleted lines, and I would rather the diff show the export's shape than my tidying of it. Say if you disagree, since you are the one reading the diff.

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