Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a prototype React renderer for A2UI, including the core rendering logic, a component library for the minimal catalog, and a gallery application for demonstration and testing. The implementation is well-structured and follows the provided design document, leveraging modern React features like useSyncExternalStore for reactivity. My review includes a few suggestions to improve type safety, package configuration, and test coverage.
renderers/react/src/adapter.tsx
Outdated
| RenderComponent: React.FC<ReactA2uiComponentProps<T>> | ||
| ) { | ||
| return function ReactWrapper({ context, buildChild }: { context: ComponentContext, buildChild: (id: string, basePath?: string) => React.ReactNode }) { | ||
| const bindingRef = useRef<ComponentBinding<T>>(null); |
There was a problem hiding this comment.
The useRef is initialized with null, but its generic type ComponentBinding<T> does not include null. With strict null checks enabled in your tsconfig.json, this will cause a type error. The ref's type should be explicitly set to ComponentBinding<T> | null to reflect its possible null value.
| const bindingRef = useRef<ComponentBinding<T>>(null); | |
| const bindingRef = useRef<ComponentBinding<T> | null>(null); |
| "peerDependencies": { | ||
| "react": "^19.2.0", | ||
| "react-dom": "^19.2.0" | ||
| }, |
There was a problem hiding this comment.
rxjs and zod are used by the library but are not listed as peerDependencies. Based on the installation instructions in README.md and the externals in vite.config.ts, they should be declared as peerDependencies to ensure that the consuming application provides compatible versions.
"peerDependencies": {
"react": "^19.2.0",
"react-dom": "^19.2.0",
"rxjs": "^7.8.1",
"zod": "^3.23.8"
}| } | ||
| }; | ||
| // resolve synchronous initial value | ||
| bindingRef.current.currentVal = Array.isArray(context.dataContext.resolveDynamicValue({ path: childList.path })) ? context.dataContext.resolveDynamicValue({ path: childList.path }) as any[] : EMPTY_ARRAY; |
There was a problem hiding this comment.
The dynamic value at childList.path is resolved twice in this line. You can store the result in a variable to avoid the redundant call and improve performance slightly.
const initialValue = context.dataContext.resolveDynamicValue({ path: childList.path });
bindingRef.current.currentVal = Array.isArray(initialValue) ? initialValue as any[] : EMPTY_ARRAY;
| const spyAddListener = vi.spyOn(context.dataContext, 'subscribeDynamicValue'); | ||
|
|
||
| type TestProps = { text?: string }; | ||
|
|
||
| const RenderTest: React.FC<{ props: TestProps }> = ({ props }) => { | ||
| return <div>{props.text}</div>; | ||
| }; | ||
|
|
||
| const TestComponent = createReactComponent<TestProps>( | ||
| (ctx) => createGenericBinding<TestProps>(ctx, []), | ||
| RenderTest as any | ||
| ); | ||
|
|
||
| const { unmount } = render(<TestComponent context={context} buildChild={() => null} />); | ||
|
|
||
| expect(spyAddListener).toHaveBeenCalled(); | ||
| // One listener added | ||
|
|
||
| unmount(); | ||
|
|
||
| // We would need a way to check if removeListener was called, but checking that the binding logic doesn't crash on unmount is a good start. | ||
| // If listeners aren't cleaned up, subsequent updates might throw if component is destroyed. | ||
| }); |
There was a problem hiding this comment.
This test for cleanup on unmount can be made more robust. You can verify that the unsubscribe function is actually called by mocking the return value of subscribeDynamicValue and spying on its unsubscribe method. This provides a more direct confirmation that cleanup is happening correctly.
const unsubscribeSpy = vi.fn();
const spyAddListener = vi.spyOn(context.dataContext, 'subscribeDynamicValue').mockReturnValue({
value: 'initial',
unsubscribe: unsubscribeSpy,
});
type TestProps = { text?: string };
const RenderTest: React.FC<{ props: TestProps }> = ({ props }) => {
return <div>{props.text}</div>;
};
const TestComponent = createReactComponent<TestProps>(
(ctx) => createGenericBinding<TestProps>(ctx, []),
RenderTest as any
);
const { unmount } = render(<TestComponent context={context} buildChild={() => null} />);
expect(spyAddListener).toHaveBeenCalled();
unmount();
expect(unsubscribeSpy).toHaveBeenCalled();
Description
This PR introduces the
@a2ui/react_prototypepackage, a React-based renderer for the A2UI protocol (v0.9). It leverages@a2ui/web_corefor the framework-agnostic data layer and provides a reactive bridge to React components usinguseSyncExternalStore.Key Features
rxjsanduseSyncExternalStore.createReactComponentutility automatically resolves A2UIDynamicValuefields (likeDynamicString) into standard JavaScript types for the React component. It also automatically generatesset<Property>callbacks (e.g.,setValue) for two-way data binding.Row,Column,List), inputs (TextField,Slider,ChoicePicker), and interactive elements (Modal,Tabs).A2uiSurfaceroot component manages the recursive construction of the UI tree, handling structural properties and child building.API Examples
Rendering a Surface
The
A2uiSurfaceis the entry point for rendering an A2UI interface. It requires aSurfaceModelinitialized with a compatible catalog.Creating Custom Components
You can extend the renderer by creating new components using
createReactComponent. The adapter handles schema validation and data-binding automatically.Two-Way Data Binding
The generic binder automatically generates setter functions for properties that accept
DynamicValues. This makes updating the data model simple.Implemented Components
The
basicCatalogcurrently includes React implementations for:Row,Column,List,Card,Divider.Text,Image,Icon,Video,AudioPlayer.Button,TextField,CheckBox,ChoicePicker,Slider,DateTimeInput.Tabs,Modal.Pre-launch Checklist