-
Notifications
You must be signed in to change notification settings - Fork 349
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
Enable ref as valid prop #862
Conversation
let button = getByRole("FancyButton", container); | ||
expect(button->innerHTML)->toBe("Click me"); |
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.
Shouldn't we test here something about the ref
? This test just shows that Button_with_ref
is rendering ok.
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.
Sure, added another assertion but didn't want to go to clicks and other events to keep the test simple and fast
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.
Thanks. (I think the expect in line 27 can be removed, because it doesn't test anything related to the ref)
…esting-libraries * '19' of github.com:/reasonml/reason-react: Add deprecations on ReactDOMTestUtils Add uri comment back on action Update src/React.re Update src/React.re Update src/React.re Snapshot with lower {}
…on-react into replace-testing-libraries * 'replace-testing-libraries' of github.com:/reasonml/reason-react: Bind React.act and React.actAsync
…on-react into ref-as-valid-prop * 'replace-testing-libraries' of github.com:/reasonml/reason-react: Bind React.act and React.actAsync Bind React.act and React.actAsync Add deprecations on ReactDOMTestUtils Add uri comment back on action Add comment on install --force Remove jest from demo/dune Update src/React.re Update src/React.re Update src/React.re Snapshot with lower {}
…lid-prop * '19' of github.com:/reasonml/reason-react: Replace react dom's testing library with ReactTestingLibrary (#859)
2d60883
to
15e0877
Compare
…ises-as-annotations * '19' of github.com:/reasonml/reason-react: Enable ref as valid prop (#862) Replace react dom's testing library with ReactTestingLibrary (#859) Add deprecations on ReactDOMTestUtils Add uri comment back on action Update src/React.re Update src/React.re Update src/React.re Snapshot with lower {}
Depends on #859, that depends on #846
Allows to have a prop with
~ref
This PR also adds the cleanup function on
ref
as a new constructor: added another "constructor" instead of changing the current callbackRef to avoid a breaking change, we might add deprecations and further cleanup in the future versions to keep those constructors to the minimum