Skip to content

docs: replace the concurrent mode note with a Suspense section - #778

Merged
tyler-reitz merged 2 commits into
FirebaseExtended:mainfrom
tyler-reitz:docs/suspense-section
Aug 5, 2026
Merged

docs: replace the concurrent mode note with a Suspense section#778
tyler-reitz merged 2 commits into
FirebaseExtended:mainfrom
tyler-reitz:docs/suspense-section

Conversation

@tyler-reitz

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

Copy link
Copy Markdown
Contributor

Closes #756.

The "Extra Experimental concurrent mode features" section rests on a claim that is no longer true. It says these features "will not be stable until sometime after React 18 is released". React 18 shipped in 2022, and concurrent mode was abandoned as a concept rather than stabilised, so a reader today is told to follow a setup guide for a React feature that does not exist.

All three of its reactjs.org/docs/concurrent-mode-* links are dead pages. This PR removes all three from the README. One more survives in a comment in example/index.tsx, left for a separate follow-up so this stays docs-scoped.

What changed

  • Removed the "Extra Experimental concurrent mode features" heading and its stale premise paragraph.
  • Added a <Suspense> bullet to What is ReactFire?, as the issue asked.
  • Rehomed the remaining content as a plain ## Suspense section.
  • Added that suspense is off by default, which the old text never stated. Verified against src/firebaseApp.tsx (suspense ?? false, lines 57 and 66).
  • Described what <SuspenseWithPerf /> actually does. It measures with the browser User Timing API, not the Firebase Performance SDK. See below.

What was kept, and why

The issue asks for the whole section to be removed. Two things were kept:

  • the suspense={true} snippet, which is the only place the repo documents how to turn suspense on
  • the <SuspenseWithPerf /> mention, which was the only prose reference to that component anywhere outside the generated API report

The stale part was the "extra experimental / concurrent mode" framing and its dead links, not the usage documentation, so that framing is what this removes.

The Performance Monitoring claim, corrected in review

The first revision carried the old text's claim that <SuspenseWithPerf /> "instruments load times with Performance Monitoring", linking the Firebase perf-mon docs. Armando checked it against the source and it is wrong: src/performance.tsx uses only performance.mark / performance.measure from the browser's User Timing API, and still carries a // TODO: Should this import firebase/performance?. Nothing there touches the Firebase Performance SDK.

The claim is inherited rather than introduced here, but this PR rewrites that sentence anyway, so it is fixed rather than carried forward. The line now describes the User Timing behaviour and links MDN.

Notes

The section claimed Suspense support would not be stable until after
React 18, and pointed at three reactjs.org/docs/concurrent-mode pages
that no longer exist. React 18 shipped in 2022 and concurrent mode was
abandoned as a concept, so the setup guide it told readers to follow is
gone.

Replaces it with a plain Suspense section, adds a bullet to What is
ReactFire, and states that suspense is off by default, which the old
text never said.

Keeps the suspense={true} snippet and the SuspenseWithPerf mention. The
stale part was the concurrent mode framing and its dead links, not the
usage documentation, and the README was the only prose reference to
either outside the generated API report.

Closes FirebaseExtended#756

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

Thanks Tyler. This reads a lot better than the old "extra experimental" framing, and getting rid of the dead concurrent-mode links is overdue. I checked the changed claims against the source and they hold up, so I am comfortable approving. Two small things, neither blocking.

What I verified

  • "Off by default" is right: FirebaseAppProvider provides the suspense context as suspense ?? false (src/firebaseApp.tsx:57), and useIsSuspenseEnabled returns the same default (:66). Good to finally have this written down.
  • "Hooks can throw promises for <Suspense> to catch" matches the code: useObservable throws observable.firstEmission (a promise) when suspense is on and there is no data yet (src/useObservable.ts:80-81).
  • The [Suspense](#suspense) bullet link resolves, since ## Suspense is the only heading of that name, and the new react.dev and example/withSuspense links are live.

One optional wording note

The new line says <SuspenseWithPerf /> "instruments load times with Performance Monitoring" and links the Firebase Performance Monitoring docs. Reading src/performance.tsx, the component measures with the browser User Timing API (performance.mark / performance.measure) and carries a // TODO: Should this import firebase/performance?, so it is not actually wired to the Firebase performance SDK. The old text linked the same page, so this is inherited rather than anything you introduced. Since you are already rewriting that sentence, it might be a natural moment to soften "Performance Monitoring" or drop the perf-mon link, but I am happy either way.

Minor, out of scope

Your description mentions removing "the last of those links from the repo". There is still one reactjs.org/docs/concurrent-mode-adoption.html link in a comment in example/index.tsx. The README is the right scope for this PR, so no need to touch it here, just flagging in case you want a quick follow-up to finish them off.

Approving. If I have misread any of the above, point me at it and I will take another look.

SuspenseWithPerf measures with the browser User Timing API
(performance.mark / performance.measure) and is not wired to the
Firebase Performance SDK; src/performance.tsx still carries a
'Should this import firebase/performance?' TODO. The old text linked
the Performance Monitoring docs, so the claim is inherited rather than
new, but this PR rewrites that sentence anyway.

Reported by Armando in review of FirebaseExtended#778.
@tyler-reitz

Copy link
Copy Markdown
Contributor Author

Thanks Armando, both findings were right and both are now addressed.

Performance Monitoring. You are correct and I took the fix rather than leaving it. src/performance.tsx measures with performance.mark / performance.measure only, and the // TODO: Should this import firebase/performance? is still sitting there, so the perf-mon link pointed at a product the component never calls. Since I was rewriting that sentence anyway, carrying the claim forward would have meant knowingly shipping it. The line now reads:

<SuspenseWithPerf /> does the same and also measures how long the fallback was shown, using the browser's User Timing API.

That is in e1dba04, a second commit rather than a force-push, so your approval stays attached to what you reviewed.

The remaining link. Also correct, and the PR body was the actual problem: it claimed this removed "the last of those links from the repo", which is false while example/index.tsx:8 still points at concurrent-mode-adoption.html. I have corrected the body to say the README, and noted the example as a follow-up. Agreed it does not belong in a docs PR.

Merging on your approval under the patch-level rule. The firebase/performance TODO is a real open question about whether that component should use the SDK at all, but that is a behaviour decision for V5, not a README fix.

@tyler-reitz
tyler-reitz merged commit afda191 into FirebaseExtended:main Aug 5, 2026
24 of 26 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.

docs: remove note about "extra experimental features"

2 participants