docs: replace the concurrent mode note with a Suspense section - #778
docs: replace the concurrent mode note with a Suspense section#778tyler-reitz wants to merge 1 commit into
Conversation
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
ba03010 to
bd23957
Compare
armando-navarro
left a comment
armando-navarro
left a comment
There was a problem hiding this comment.
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:
FirebaseAppProviderprovides the suspense context assuspense ?? false(src/firebaseApp.tsx:57), anduseIsSuspenseEnabledreturns the same default (:66). Good to finally have this written down. - "Hooks can throw promises for
<Suspense>to catch" matches the code:useObservablethrowsobservable.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## Suspenseis the only heading of that name, and the newreact.devandexample/withSuspenselinks 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.
2 participants
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 the last of those links from the repo.What changed
<Suspense>bullet to What is ReactFire?, as the issue asked.## Suspensesection.src/firebaseApp.tsx(suspense ?? false, lines 57 and 66).7 lines added, 9 removed. No behaviour claims changed other than the default, which was previously undocumented.
What was kept, and why
The issue asks for the whole section to be removed. Two things were kept:
suspense={true}snippet, which is the only place the repo documents how to turn suspense on<SuspenseWithPerf />mention, which was the only prose reference to that component anywhere outside the generated API reportThe stale part was the "extra experimental / concurrent mode" framing and its dead links, not the usage documentation, so that framing is what this removes.
Notes
Test Node.js 24goes red here, it is very likely CI: test/firestore.test.tsx flakes on Node 24 with RESOURCE_EXHAUSTED on the Listen stream #776 rather than anything in this diff, given this PR touches onlyREADME.md.