findBySelector in ReactTestUtils is typed incorrectly

[FaustXVI]

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 (https://github.com/reasonml/reason-react/blob/d0cab5962da2d00e9f13918f000d9ebbd11bbb28/src/ReactTestUtils.re#L88) is at the origin of the problem.

Am I correct or am I missunderstanding / missusing something ?

Thanks

[FaustXVI]

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.

[davesnx]

Since Melange transforming None to undefined isn't an issue anymore. Closing.