Add support for testing React components #209
Conversation
remorses
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for testing React components by enabling the use of a custom TestRenderer with the existing render function and implementing proper await mechanisms for React's asynchronous rendering. The changes allow tests to wait for React commits to complete before assertions.
Key changes:
- Modified the render function to accept either a CliRendererConfig or CliRenderer instance
- Added callback support to the reconciler to notify when rendering is complete
- Extended TestRenderer with ready Promise for awaiting render completion
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/reconciler/renderer.ts | Modified render function to accept TestRenderer and added Promise-based completion handling |
| packages/react/src/reconciler/renderer.test.tsx | Added comprehensive test suite demonstrating React component testing with state updates |
| packages/react/src/reconciler/reconciler.ts | Added optional callback parameter to _render function |
| packages/react/src/reconciler/host-config.ts | Added TestRenderer ready Promise management in resetAfterCommit |
| packages/core/src/testing/test-renderer.ts | Extended TestRenderer interface and implementation with ready Promise support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if ("resolveReady" in renderer && "ready" in renderer) { | ||
| const testRenderer = renderer as TestRenderer |
There was a problem hiding this comment.
The type checking using 'in' operator followed by type assertion is fragile. Consider using a proper type guard function or checking if renderer is an instance of TestRenderer.
| if ("resolveReady" in containerInfo.ctx && "ready" in containerInfo.ctx) { | ||
| const testRenderer = containerInfo.ctx as TestRenderer |
There was a problem hiding this comment.
Duplicate type checking pattern from renderer.ts. This logic should be extracted into a reusable type guard function to avoid code duplication.
|
|
||
| const renderer = new CliRenderer(ziglib, rendererPtr, stdin, stdout, width, height, config) | ||
|
|
||
| const renderer = new CliRenderer(ziglib, rendererPtr, stdin, stdout, width, height, config) as TestRenderer |
There was a problem hiding this comment.
Type assertion 'as TestRenderer' is unsafe. The CliRenderer instance doesn't actually have the ready and resolveReady properties at this point. Consider properly extending the class or using Object.assign to add the properties.
| const renderer = new CliRenderer(ziglib, rendererPtr, stdin, stdout, width, height, config) as TestRenderer | |
| const renderer = Object.assign( | |
| new CliRenderer(ziglib, rendererPtr, stdin, stdout, width, height, config), | |
| { ready: undefined, resolveReady: undefined } | |
| ) as TestRenderer |
|
i'm not sure on this one... i had started looking into changing the api for this previously but didn't finish it! i was leaning towards exposing a Something like: import { createCliRenderer } from "@opentui/core";
import { createRoot } from "@opentui/react";
const renderer = await createCliRenderer(<config>);
createRoot(renderer).render(<App />); |
|
That makes sense. I also have some issues creating the promise for ready in a way that does not create issues with Solid |
2 participants
Added support for passing a custom renderer in React render and make it wait until React commits
I added a ready field to the TestRenderer to be able to await the React commit. Because React renders asynchronously