-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
useSelect: add unit tests for static select mode #46606
Conversation
Size Change: +7 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks!
I hope you don't mind I used the opportunity to refactor fireEvent
usage to user-event
.
🚀
I actually do mind in this particular case 🙂 These are very low-level tests, some of them try to reproduce precise timing of events -- like a store update between render and effect -- and I don't want any Can you please revert the commit? |
I tend to disagree because while I understand your rationale, I consider the |
This reverts commit f97baf6.
That's true, in a realistic UI a click wouldn't happen this way but for this particular test suite it's not relevant. We're doing the clicks just as an easy way to trigger something inside the component. Doing a state update, or calling a function returned from a hook. We could achieve the goal also by assigning the let innerSetFoo;
function TestComponent() {
const [ foo, setFoo ] = useState( 'foo' );
innerSetFoo = setFoo;
return <div>{ foo }</div>;
}
render( <TestComponent /> );
expect( screen ).toHaveTextContent( 'foo' );
act( () => innerSetFoo( 'bar' ) );
expect( screen ).toHaveTextContent( 'bar' ); |
When working on new
useSelect
implementation in #46538 I discovered we don't have any unit tests for the "static select mode", i.e., usinguseSelect
in a non-reactive way, typically to read the latest value in an event handler:This PR fixes that omission. There is a new test that passes with the old
useSelect
, and will also pass with the newuseSelect
in #46538. Test-driven development!