refactor(react)!: delete the React basic catalog implementation - #2630
josemontespg wants to merge 1 commit into
Conversation
6d5b331 to
a86b4ae
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the React renderer by removing local React implementations of basic catalog components in favor of web_core's basic catalog Custom Elements, updating imports and tests accordingly. To avoid a race condition where Lit components render on their first paint before the markdown renderer is set, it is recommended to use useLayoutEffect instead of useEffect in A2uiSurface to synchronously configure the markdown renderer during React's commit phase.
| */ | ||
|
|
||
| import React, {useCallback, useMemo, useSyncExternalStore} from 'react'; | ||
| import React, {useCallback, useEffect, useMemo, useSyncExternalStore} from 'react'; |
There was a problem hiding this comment.
Import useLayoutEffect instead of useEffect to avoid race conditions with Lit's asynchronous rendering cycle.
| import React, {useCallback, useEffect, useMemo, useSyncExternalStore} from 'react'; | |
| import React, {useCallback, useLayoutEffect, useMemo, useSyncExternalStore} from 'react'; |
| useEffect(() => { | ||
| setMarkdownRenderer(markdownRenderer); | ||
| }, [markdownRenderer]); |
There was a problem hiding this comment.
Using useEffect to set the global markdown renderer can lead to a race condition with Lit's asynchronous update cycle. Lit components schedule their updates as microtasks during their connectedCallback (which runs synchronously when React inserts them into the DOM). Since useEffect runs asynchronously in a macrotask after the paint, Lit's first update cycle will execute before useEffect runs and sets the markdown renderer. This can cause the components to render without the markdown renderer on their first paint.
Using useLayoutEffect ensures that setMarkdownRenderer is called synchronously during React's commit phase, before the microtask queue flushes and before the browser paints, guaranteeing that the markdown renderer is available for Lit's initial render.
| useEffect(() => { | |
| setMarkdownRenderer(markdownRenderer); | |
| }, [markdownRenderer]); | |
| useLayoutEffect(() => { | |
| setMarkdownRenderer(markdownRenderer); | |
| }, [markdownRenderer]); |
60c1bf8 to
7646f12
Compare
7646f12 to
4aee1d8
Compare
4aee1d8 to
e40022f
Compare
e40022f to
8d28391
Compare
0cf7244 to
83db1f3
Compare
83db1f3 to
cfa8f1a
Compare
cfa8f1a to
8234a11
Compare
8234a11 to
4f468bd
Compare
4f468bd to
689d3c3
Compare
689d3c3 to
1e3be23
Compare
1e3be23 to
4641c24
Compare
4641c24 to
2250ba9
Compare
2250ba9 to
d19c175
Compare
The v0_9 renderer renders the basic catalog through the Web Components in @a2ui/web_core/v0_9/basic_catalog, so the 23 duplicated React implementations are removed along with their tests. Consumers import basicCatalog and the individual components from @a2ui/web_core/v0_9/basic_catalog. MarkdownContext and useMarkdownRenderer keep their names and behaviour: A2uiSurface forwards the context value into web_core's setMarkdownRenderer. The basic catalog no longer server-renders, since custom elements produce no markup outside a browser.
d19c175 to
9412028
Compare
The v0_9 React renderer renders the basic catalog through the Web Components in
@a2ui/web_core/v0_9/basic_catalog, so the 23 duplicated React implementations are removed along with their unit tests.What changes for consumers
Import the catalog and the individual components from web_core:
MarkdownContextanduseMarkdownRendererkeep their names, their behaviour, and their import path.A2uiSurfaceforwards the context value into web_core'ssetMarkdownRenderer, mirroring what the Angular renderer does, so markdown wiring in consumer code does not change.Custom React components are unaffected:
createComponentImplementationstill returns a React implementation, and the renderer still renders it through React.Breaking
@a2ui/react/v0_9no longer exportsbasicCatalog,Text,Button,Card, or the other basic components.Testing
renderers/reactunit tests pass;integration-scenarios.test.tsxnow drives the web_core catalog end to end and waits for Lit's update cycle.