Js.String.match should probably be (Js_re.t, Js.String.t) => option(array(Js.Nullable.t(Js.String.t)))

[renatoalencar]

Thank you for filing! Check list:

• Is it a bug? Usage questions should often be asked in the forum instead.
• Concise, focused, friendly issue title & description.
• A minimal, reproducible example.
  • Ideally in OCaml syntax for now. To isolate the problem caused by either Reason or the New Syntax.
• OS and browser versions, if relevant.
• Is it already fixed in master?

Let's consider this example in Reason bellow:

let m = Js.String.match([%re "/hello (world)?/"], "hello word");
Js.log(m);

It compiles to the code bellow.

// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';

var Caml_option = require("bs-platform/lib/js/caml_option.js");

var m = "hello word".match(/hello (world)?/);

var m$1 = m === null ? undefined : Caml_option.some(m);

console.log(m$1);

exports.m = m$1;
/* m Not a pure module */

When I run that, it gives me the following result:

[
  'hello ',
  undefined,
  index: 0,
  input: 'hello word',
  groups: undefined
]

Since the capture group is not found, it gives me an undefined value instead of not appearing in the result array. So I would have to filter that out, either wrapping everything as an option or using Js.Nullable.isNullable. The thing is that this is not possible since the array values don't have a valid type for that function.

In order to actually reproduce the issue, try something like this:

let m = Js.String.match([%re "/hello (world)?/"], "hello word");

let n =
  switch (m) {
  | Some(n) => Js.Array.filter(a => !Js.Nullable.isNullable(a), n)
  | None => []
  };

Js.log(n);

This gives me the following error:

[bobzhang]

this seems to be a bug in the bindings, would you send a PR?

[renatoalencar]

this seems to be a bug in the bindings, would you send a PR?

Sure, I would love to do that.

[renatoalencar]

@bobzhang This change also changed a bunch of other files, should I commit that?