Skip to content

docs(example): drop the concurrent mode framing from the demo entry - #781

Open
tyler-reitz wants to merge 1 commit into
FirebaseExtended:mainfrom
tyler-reitz:docs/example-concurrent-mode
Open

docs(example): drop the concurrent mode framing from the demo entry#781
tyler-reitz wants to merge 1 commit into
FirebaseExtended:mainfrom
tyler-reitz:docs/example-concurrent-mode

Conversation

@tyler-reitz

Copy link
Copy Markdown
Contributor

Follow-up to #778, which removed this same obsolete premise from the README but left this copy of it. Refs #756.

What was wrong

example/index.tsx told readers:

You'll need to use an experimental build of React to use Concurrent mode
https://reactjs.org/docs/concurrent-mode-adoption.html#installation

React 18 shipped in 2022 and concurrent mode was abandoned as a concept rather than stabilised, so that instruction cannot be followed and the link is a dead page. The two commented-out react/experimental and react-dom/experimental imports existed only to serve that premise, so they go with it.

This removes the last reactjs.org reference in the repository. #778's description originally claimed to have done that, which was wrong, and Armando caught it. This is the correction.

Scope

Comments only. No active code changed, so the demo behaves identically. 5 lines added, 6 removed.

I deliberately did not fix two related things I found while in here, because both are more than a comment edit. Pricing them rather than leaving them vague:

1. The example uses the legacy render API, and the commented-out "concurrent" path uses the modern one. The active code calls ReactDOM.render, deprecated in React 18. The commented-out block calls ReactDOM.createRoot, which is now simply the standard API and not experimental at all. So the file currently has it backwards: the path labelled experimental is the current one. Fixing this is a real change to the demo, roughly a 5 line diff plus actually running the example to confirm it still works, and it deserves its own PR rather than riding along in a comment cleanup.

2. The identifiers are still named for concurrent mode. NonConcurrentModeApp and ConcurrentModeApp are local import aliases in this one file, so renaming them to something like AppWithoutSuspense and AppWithSuspense is about 3 lines and affects nothing outside it. I left them because this PR touches no active code and I would rather that stay true. Worth doing alongside item 1.

Neither is urgent. Both are cheap. Flagging them so they are a decision rather than something that quietly rots for another four years.

Not addressed

Whether the withSuspense demo path should be revived, rewritten or deleted is a genuine question and not a docs edit. It has been commented out long enough that nobody knows if it runs. That belongs with the V5 Suspense work, since the suspend path moves to use() there anyway.

Follow-up to FirebaseExtended#778, which removed the same obsolete premise from the
README but left this copy of it.

The comment told readers they need "an experimental build of React to
use Concurrent mode" and linked
reactjs.org/docs/concurrent-mode-adoption.html, a dead page. React 18
shipped in 2022 and concurrent mode was abandoned as a concept rather
than stabilised, so the instruction could not be followed. The two
commented-out react/experimental and react-dom/experimental imports
existed only to serve that premise and go with it.

Comments only. No active code changes, so the demo behaves identically.

This removes the last reactjs.org reference in the repository.

Refs FirebaseExtended#756
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.

1 participant