Core `Map.get` when map has with optional values

[angusjf]

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.
• OS and browser versions, if relevant.
• Is it already fixed in master? Instructions

type key = A | B

let dict = Map.fromArray([(A, Some(1)), (B, None)])

assert(dict->Map.get(A) == Some(Some(1))) // as expected

assert(dict->Map.get(B) == None)          // expected to fail, passes

assert(dict->Map.get(B) == Some(None))    // expected to pass, fails

JS's Map returns undefined when the key is not present, and also when the value is undefined.

https://github.com/rescript-lang/experimental-rescript-stdlib-build/blob/aa8001a778a8975f83ece0a5266b3049b9d20d42/core/src/Core__Map.res#L14

Apologies if issue is in the wrong place, I am new to rescript

[cometkim]

Good catch. I think it's an issue of the Core binding

It actually need runtime to handle nested option correctly

module Map = {
  include Map

  let get = (t, key) => {
    if t->has(key) {
      Some(t->get(key)->Option.getUnsafe)
    } else {
      None
    }
  }
}

[zth]

You should be able to work around this by using Null.t instead of option.

[bloodyowl]

yeah, otherwise the get would need a runtime wrapper (like map.has(x) ? Some(map.get(x)) : None)

[cknitt]

See also rescript-lang/rescript-core#217

[cknitt]

@return(undefined_to_opt) can be added to a binding to ensure correct handling of nesting options.

But the question is if we should get rid of nested options altogether? See #7430.

[cknitt]

@return(undefined_to_opt) can be added to a binding to ensure correct handling of nesting options.

Hmm, actually not, it will emit code like

let v = m.get("A");

let v$1 = v === undefined ? undefined : Primitive_option.some(v);

which does not really help us distinguish between the value being absent and the value being present, but undefined.