Pass undefined for an optional external argument or JSX key that is None

changes/optional-external-argument-none.fix.md

@@ -0,0 +1 @@
+- Fix a `TypeError` when an optional external argument such as `~x=?dict->Dict.get(k)`, or an optional JSX key, is `None`.

compiler/core/js_of_lam_option.ml

@@ -77,7 +77,9 @@ let get_default_undefined_from_optional (arg : J.expression) : J.expression =
       if Js_analyzer.is_okay_to_duplicate arg then
         (* FIXME: no need do such inlining*)
         E.econd (is_not_none arg) (val_from_option arg) E.undefined
-      else E.runtime_call Primitive_modules.option "valFromOption" [arg]
+      else
+        (* [valFromOption] returns [undefined] for [None] as well *)
+        E.runtime_call Primitive_modules.option "valFromOption" [arg]
 
 let option_unwrap (arg : J.expression) : J.expression =
   let desc = arg.expression_desc in

packages/@rescript/runtime/Primitive_option.res

@@ -63,7 +63,7 @@ let fromNull = (type t, x: Primitive_js_extern.null<t>): option<t> =>
 /** Preserves `None`/`undefined` and unwraps one outer option layer.
     Nested `Some` values stay encoded one level deeper. */
 let valFromOption = (x: Obj.t): Obj.t =>
-  if x !== Obj.repr(Primitive_js_extern.null) && isNested(x) {
+  if !Primitive_js_extern.testAny(x) && isNested(x) {
     let {depth}: nested = Obj.magic(x)
     if depth == 0 {
       Obj.magic(None)

packages/@rescript/runtime/lib/es6/Primitive_option.mjs

@@ -46,7 +46,7 @@ function fromNull(x) {
 }
 
 function valFromOption(x) {
-  if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
+  if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
     return x;
   }
   let depth = x.BS_PRIVATE_NESTED_SOME_NONE;

packages/@rescript/runtime/lib/js/Primitive_option.cjs

@@ -46,7 +46,7 @@ function fromNull(x) {
 }
 
 function valFromOption(x) {
-  if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
+  if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
     return x;
   }
   let depth = x.BS_PRIVATE_NESTED_SOME_NONE;

tests/tests/src/optional_external_arg_none_test.mjs

@@ -0,0 +1,148 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+import * as Mocha from "mocha";
+import * as Test_utils from "./test_utils.mjs";
+import * as Primitive_option from "@rescript/runtime/lib/es6/Primitive_option.mjs";
+import * as JsxRuntime from "react/jsx-runtime";
+
+function greet(name) {
+  return name === undefined ? "Hello" : "Hello " + name
+}
+
+function Greeting(name) {
+  this.text = greet(name)
+}
+;
+
+let makeTarget = (() => {
+  const calls = []
+  const record = (...args) => {
+    calls.push(args.map(String).join(" "))
+  }
+  return { calls, greet, setLevel: record, setMode: record }
+});
+
+let opaque = (x => x);
+
+function greetUser(names, id) {
+  return greet(Primitive_option.valFromOption(names[id]));
+}
+
+function hello(name, param) {
+  return greet(name !== undefined ? Primitive_option.valFromOption(name) : undefined);
+}
+
+function helloUser(names, id) {
+  return greet(Primitive_option.valFromOption(names[id]));
+}
+
+function item(keys, id) {
+  return JsxRuntime.jsx("li", {
+    children: id
+  }, Primitive_option.valFromOption(keys[id]));
+}
+
+Mocha.describe("Optional_external_arg_none_test", () => {
+  Mocha.test("None from a call, a lookup or a conditional", () => {
+    let names = {
+      ann: "Ann"
+    };
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 68, characters 7-14", "Hello Ann", greetUser(names, "ann"));
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 69, characters 7-14", "Hello", greetUser(names, "bob"));
+    let list = ["Ann"];
+    let at = i => greet(Primitive_option.valFromOption(list[i]));
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 72, characters 7-14", [
+      "Hello Ann",
+      "Hello"
+    ], [
+      at(0),
+      at(1)
+    ]);
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 73, characters 7-14", "Hello", greet(Primitive_option.valFromOption(opaque(undefined))));
+    let pick = (b, name) => greet(Primitive_option.valFromOption(b ? name : undefined));
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 75, characters 7-14", [
+      "Hello Ann",
+      "Hello"
+    ], [
+      pick(true, "Ann"),
+      pick(false, "Ann")
+    ]);
+  });
+  Mocha.test("None passed to @send and @new externals", () => {
+    let t = makeTarget();
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 80, characters 7-14", "Hello", t.greet(Primitive_option.valFromOption(opaque(undefined))));
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 81, characters 7-14", "Hello", new Greeting(Primitive_option.valFromOption(opaque(undefined))).text);
+  });
+  Mocha.test("None forwarded by an inlined function", () => {
+    let names = {
+      ann: "Ann"
+    };
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 86, characters 7-14", [
+      "Hello Ann",
+      "Hello"
+    ], [
+      helloUser(names, "ann"),
+      helloUser(names, "bob")
+    ]);
+  });
+  Mocha.test("None for @int and @string arguments", () => {
+    let t = makeTarget();
+    let levels = ["high"];
+    let modes = ["dark"];
+    t.setLevel((() => {
+      switch (Primitive_option.valFromOption(levels[0])) {
+        case "low" :
+          return 0;
+        case "high" :
+          return 10;
+      }
+    })());
+    t.setLevel((() => {
+      switch (Primitive_option.valFromOption(levels[1])) {
+        case "low" :
+          return 0;
+        case "high" :
+          return 10;
+      }
+    })());
+    t.setMode((() => {
+      switch (Primitive_option.valFromOption(modes[0])) {
+        case "light" :
+          return "light-mode";
+        case "dark" :
+          return "dark-mode";
+      }
+    })());
+    t.setMode((() => {
+      switch (Primitive_option.valFromOption(modes[1])) {
+        case "light" :
+          return "light-mode";
+        case "dark" :
+          return "dark-mode";
+      }
+    })());
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 97, characters 7-14", [
+      "10",
+      "undefined",
+      "dark-mode",
+      "undefined"
+    ], t.calls);
+  });
+  Mocha.test("a JSX key that is None", () => {
+    let keys = {
+      a: "key-a"
+    };
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 102, characters 7-14", "key-a", Primitive_option.fromNullable(item(keys, "a").key));
+    Test_utils.eq("File \"optional_external_arg_none_test.res\", line 103, characters 7-14", undefined, Primitive_option.fromNullable(item(keys, "b").key));
+  });
+});
+
+export {
+  makeTarget,
+  opaque,
+  greetUser,
+  hello,
+  helloUser,
+  item,
+}
+/*  Not a pure module */

tests/tests/src/optional_external_arg_none_test.res

@@ -0,0 +1,105 @@
+@@config({flags: ["-bs-jsx", "4"]})
+
+open Mocha
+open Test_utils
+
+// An optional argument of an external passed as `~x=?e` is left out, that
+// is passed as undefined, when `e` is None, and so is a JSX key. A call, a
+// lookup or a conditional cannot be read twice to test it first, so it is
+// unwrapped by Primitive_option.valFromOption, which keeps undefined.
+
+%%raw(`
+function greet(name) {
+  return name === undefined ? "Hello" : "Hello " + name
+}
+
+function Greeting(name) {
+  this.text = greet(name)
+}
+`)
+
+@val external greet: (~name: string=?, unit) => string = "greet"
+
+type greeting
+@new external makeGreeting: (~name: string=?, unit) => greeting = "Greeting"
+@get external text: greeting => string = "text"
+
+type target
+
+let makeTarget: unit => target = %raw(`() => {
+  const calls = []
+  const record = (...args) => {
+    calls.push(args.map(String).join(" "))
+  }
+  return { calls, greet, setLevel: record, setMode: record }
+}`)
+
+@get external calls: target => array<string> = "calls"
+
+@send external greetWith: (target, ~name: string=?, unit) => string = "greet"
+
+@send
+external setLevel: (target, ~level: @int [#low | @as(10) #high]=?, unit) => unit = "setLevel"
+
+@send
+external setMode: (
+  target,
+  ~mode: @string [@as("light-mode") #light | @as("dark-mode") #dark]=?,
+  unit,
+) => unit = "setMode"
+
+// Opaque, so that the optimizer cannot tell the argument is None.
+let opaque: 'a => 'a = %raw(`x => x`)
+
+let greetUser = (names: dict<string>, id) => greet(~name=?names->Dict.get(id), ())
+
+// Inlined into its callers, where the argument replaces [name].
+let hello = (~name=?, ()) => greet(~name?, ())
+
+let helloUser = (names: dict<string>, id) => hello(~name=?names->Dict.get(id), ())
+
+let item = (keys: dict<string>, id) => <li key=?{keys->Dict.get(id)}>{React.string(id)}</li>
+
+@get external elementKey: React.element => Nullable.t<string> = "key"
+
+describe(__MODULE__, () => {
+  test("None from a call, a lookup or a conditional", () => {
+    let names = dict{"ann": "Ann"}
+    eq(__LOC__, "Hello Ann", greetUser(names, "ann"))
+    eq(__LOC__, "Hello", greetUser(names, "bob"))
+    let list = ["Ann"]
+    let at = i => greet(~name=?list->Array.get(i), ())
+    eq(__LOC__, ("Hello Ann", "Hello"), (at(0), at(1)))
+    eq(__LOC__, "Hello", greet(~name=?opaque(None), ()))
+    let pick = (b, name) => greet(~name=?b ? Some(name) : None, ())
+    eq(__LOC__, ("Hello Ann", "Hello"), (pick(true, "Ann"), pick(false, "Ann")))
+  })
+
+  test("None passed to @send and @new externals", () => {
+    let t = makeTarget()
+    eq(__LOC__, "Hello", t->greetWith(~name=?opaque(None), ()))
+    eq(__LOC__, "Hello", makeGreeting(~name=?opaque(None), ())->text)
+  })
+
+  test("None forwarded by an inlined function", () => {
+    let names = dict{"ann": "Ann"}
+    eq(__LOC__, ("Hello Ann", "Hello"), (helloUser(names, "ann"), helloUser(names, "bob")))
+  })
+
+  test("None for @int and @string arguments", () => {
+    let t = makeTarget()
+    let levels = [#high]
+    let modes = [#dark]
+    t->setLevel(~level=?levels->Array.get(0), ())
+    t->setLevel(~level=?levels->Array.get(1), ())
+    t->setMode(~mode=?modes->Array.get(0), ())
+    t->setMode(~mode=?modes->Array.get(1), ())
+    eq(__LOC__, ["10", "undefined", "dark-mode", "undefined"], calls(t))
+  })
+
+  test("a JSX key that is None", () => {
+    let keys = dict{"a": "key-a"}
+    eq(__LOC__, Some("key-a"), item(keys, "a")->elementKey->Nullable.toOption)
+    eq(__LOC__, None, item(keys, "b")->elementKey->Nullable.toOption)
+  })
+})