Skip to content
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

Merged
merged 26 commits into from
Nov 25, 2024
Merged

Enable ref as valid prop #862

merged 26 commits into from
Nov 25, 2024

Conversation

davesnx
Copy link
Member

@davesnx davesnx commented Nov 20, 2024

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

Comment on lines +26 to +27
let button = getByRole("FancyButton", container);
expect(button->innerHTML)->toBe("Click me");
Copy link
Collaborator

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.

Copy link
Member Author

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

Copy link
Collaborator

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)

@anmonteiro
Copy link
Member

…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 {}
Base automatically changed from replace-testing-libraries to 19 November 25, 2024 17:36
…lid-prop

* '19' of github.com:/reasonml/reason-react:
  Replace react dom's testing library with ReactTestingLibrary (#859)
@davesnx davesnx merged commit 08f5e19 into 19 Nov 25, 2024
3 checks passed
@davesnx davesnx deleted the ref-as-valid-prop branch November 25, 2024 18:01
davesnx added a commit that referenced this pull request Nov 25, 2024
…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 {}
@davesnx davesnx mentioned this pull request Nov 25, 2024
davesnx added a commit that referenced this pull request Nov 25, 2024
* '19' of github.com:/reasonml/reason-react:
  Enable ref as valid prop (#862)
  Replace react dom's testing library with ReactTestingLibrary (#859)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants