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
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
35d588c
Install melange-testing-library
davesnx Nov 18, 2024
1cd6a6b
Install melange-testing-library npm deps
davesnx Nov 18, 2024
05aa7e1
Vendor melange-testing-library
davesnx Nov 18, 2024
4434bfd
Fix Form__test with RTL
davesnx Nov 18, 2024
ab43c4b
Start migrating Hooks__test
davesnx Nov 19, 2024
8234b69
Remove dependency
davesnx Nov 19, 2024
49b35e9
Remove unused code from Form__test
davesnx Nov 19, 2024
3ca3ba1
Add a jest-devtoolsgs
davesnx Nov 19, 2024
41ba9c2
Add a jest-devtools
davesnx Nov 19, 2024
c7998a9
Migrate Hooks and Form into RTL
davesnx Nov 19, 2024
5e05f2e
Add demo to manually test easily
davesnx Nov 19, 2024
f8a08c3
Use Uncurried for tests
davesnx Nov 19, 2024
8d156d7
Migrate all React__test
davesnx Nov 19, 2024
33ad56a
Force install since we are dealing with R19
davesnx Nov 19, 2024
30d82df
Snapshot with lower {}
davesnx Nov 19, 2024
507b0fb
Enable ref in ppx
davesnx Nov 20, 2024
f3dc9f3
Add jest test for ref
davesnx Nov 20, 2024
80e85ae
Remove jest from demo/dune
davesnx Nov 20, 2024
b39168f
Add comment on install --force
davesnx Nov 20, 2024
0470e59
Improve test from checking ref
davesnx Nov 20, 2024
7aaef3e
Merge branch '19' of github.com:/reasonml/reason-react into replace-t…
davesnx Nov 25, 2024
5171ac5
Bind React.act and React.actAsync
davesnx Nov 25, 2024
ad36fbb
Bind React.act and React.actAsync
davesnx Nov 25, 2024
52addbc
Merge branch 'replace-testing-libraries' of github.com:/reasonml/reas…
davesnx Nov 25, 2024
75e2d7c
Merge branch 'replace-testing-libraries' of github.com:/reasonml/reas…
davesnx Nov 25, 2024
15e0877
Merge branch '19' of github.com:/reasonml/reason-react into ref-as-va…
davesnx Nov 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions ppx/reason_react_ppx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -653,11 +653,6 @@ let jsxMapper =
(Invalid_argument
"Key cannot be accessed inside of a component. Don't worry - you \
can always key a component from its parent!")
| Pexp_fun (Labelled "ref", _, _, _) | Pexp_fun (Optional "ref", _, _, _) ->
raise
(Invalid_argument
"Ref cannot be passed as a normal prop. Please use `forwardRef` \
API instead.")
| Pexp_fun
( ((Optional label | Labelled label) as arg),
default,
Expand Down
7 changes: 7 additions & 0 deletions ppx/test/component.t/input.re
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ module Forward_Ref = {
});
};

module Ref_as_prop = {
[@react.component]
let make = (~children, ~ref) => {
<button ref className="FancyButton"> children </button>;
};
};

module Onclick_handler_button = {
[@react.component]
let make = (~name, ~isDisabled=?) => {
Expand Down
21 changes: 21 additions & 0 deletions ppx/test/component.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@ We need to output ML syntax here, otherwise refmt could not parse it.
make ~buttonRef:(Props ## buttonRef) ~children:(Props ## children) in
Output$Forward_Ref)
end
module Ref_as_prop =
struct
external makeProps :
children:'children ->
ref:'ref ->
?key:string -> unit -> < children: 'children ;ref: 'ref > Js.t
= ""[@@mel.obj ]
let make =
((fun ~children ->
((fun ~ref ->
ReactDOM.jsx "button"
(((ReactDOM.domProps)[@merlin.hide ]) ~children ~ref
~className:"FancyButton" ()))
[@warning "-16"]))
[@warning "-16"])
let make =
let Output$Ref_as_prop
(Props : < children: 'children ;ref: 'ref > Js.t) =
make ~ref:(Props ## ref) ~children:(Props ## children) in
Output$Ref_as_prop
end
module Onclick_handler_button =
struct
external makeProps :
Expand Down
35 changes: 35 additions & 0 deletions test/Ref__test.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
open Jest;
open Expect;

module Button_with_ref = {
[@react.component]
let make = (~children, ~ref) => {
<button ref role="FancyButton"> children </button>;
};
};

let getByRole = (role, container) => {
ReactTestingLibrary.getByRole(~matcher=`Str(role), container);
};

[@mel.get] external innerHTML: Dom.element => string = "innerHTML";

describe("ref", () => {
test("can be passed as prop to a component", () => {
let domRef = React.createRef();
let container =
ReactTestingLibrary.render(
<Button_with_ref ref={ReactDOM.Ref.domRef(domRef)}>
{React.string("Click me")}
</Button_with_ref>,
);
let button = getByRole("FancyButton", container);
expect(button->innerHTML)->toBe("Click me");
Comment on lines +26 to +27
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)

let content =
switch (Js.Nullable.toOption(domRef.current)) {
| Some(element) => element->innerHTML
| None => failwith("No element found")
};
expect(content)->toBe("Click me");
})
});
Loading