findBySelector in ReactTestUtils is typed incorrectly
FaustXVI opened this issue · 2 comments
I am new to reason and react so I may be completely wrong but it seems to me like DOM.findBySelector is typed as returning an option where it should actually return a Js.Nullable.
Indeed, when not found, in javascript (and thus in reason) findBySelector returns null
but None
in reason is the equivalant of undefined
.
This leads to wrong behavior like in the following test :
open Jest;
open ReactTestUtils;
describe("My basic test", () => {
let refContainer = ref(None);
beforeEach(prepareContainer(refContainer));
afterEach(cleanupContainer(refContainer));
test("fails because it doesn't contain a button", () => {
let container = getContainer(refContainer);
act(() => {ReactDOMRe.render(<div />, container)});
let button = DOM.findBySelector(container, "button");
switch (button) {
| None => fail("Button not found")
| Some(_) => pass
};
});
});
My understanding so far is that the querySelector (
reason-react/src/ReactTestUtils.re
Line 88 in d0cab59
Am I correct or am I missunderstanding / missusing something ?
Thanks
After investigating a bit more, I think that the querySelector
is missing the [@bs.return nullable]
annotation. Then the code would behave as expected.
Here is the same test with the correct behaviour :
open Jest;
open ReactTestUtils;
[@bs.send] [@bs.return nullable]
external querySelector: (Dom.element, string) => option(Dom.element) =
"querySelector";
describe("My basic test", () => {
let refContainer = ref(None);
beforeEach(prepareContainer(refContainer));
afterEach(cleanupContainer(refContainer));
test("fails because it doesn't contain a button", () => {
let container = getContainer(refContainer);
act(() => {ReactDOMRe.render(<div />, container)});
let button = querySelector(container, "button");
switch (button) {
| None => fail("Refresh button not found")
| Some(_) => pass
};
});
});
If I am correct with this fix, I'll be happy to do a PR.
Since Melange transforming None to undefined isn't an issue anymore. Closing.