[noop] Typecheck react-noop-renderer against host config and renderer API#644
Conversation
6af11f2 to
807c8de
Compare
Greptile SummaryThis PR sets up proper Flow typechecking for the
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: e7193ce |
| // $FlowFixMe[prop-missing] | ||
| boundary.status = 'client-render'; |
There was a problem hiding this comment.
Bug: boundary.status should be boundary.state
The SuspenseInstance type (line 37-40) defines a state property, not status. The analogous method writeCompletedBoundaryInstruction (line 310) correctly uses boundary.state = 'complete', but this line incorrectly uses boundary.status. The $FlowFixMe[prop-missing] suppression hides a real bug rather than just a harmless type mismatch.
Since the goal of this PR is to set up typechecking for the noop renderer, this is an opportunity to fix the property name rather than suppress it:
| // $FlowFixMe[prop-missing] | |
| boundary.status = 'client-render'; | |
| // $FlowFixMe[prop-missing] | |
| boundary.state = 'client-render'; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-noop-renderer/src/ReactNoopServer.js
Line: 319-320
Comment:
**Bug: `boundary.status` should be `boundary.state`**
The `SuspenseInstance` type (line 37-40) defines a `state` property, not `status`. The analogous method `writeCompletedBoundaryInstruction` (line 310) correctly uses `boundary.state = 'complete'`, but this line incorrectly uses `boundary.status`. The `$FlowFixMe[prop-missing]` suppression hides a real bug rather than just a harmless type mismatch.
Since the goal of this PR is to set up typechecking for the noop renderer, this is an opportunity to fix the property name rather than suppress it:
```suggestion
// $FlowFixMe[prop-missing]
boundary.state = 'client-render';
```
How can I resolve this? If you propose a fix, please make it concise....like for the other renderers
|
Upstream PR was closed or merged. Code is synced via branch mirror. |
Mirror of facebook/react#35944
Original author: eps1lon
The
react-noop-rendererwas not actually typechecked against the host config and renderer API. This missed a bunch of bugs due to wrong argument or missing host config.Not implementing certain host configs should be done explicitly (e.g. throwing) instead of just omission. This makes it clearer why certain tests can't be written against the noop renderer.
This PR only sets up typechecking by adding a new inlined host config for the
react-noop-rendererpackage. No runtime behavior should be changed. In follow-ups, I'll adjust the runtime to fix the type issues.With types, setting up test infra for new features is easier (e.g. writing
ViewTransitiontests against the noop renderer).Test plan