rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

@as support for @obj makes inconsistent conversion

Open mununki opened this issue 2 years ago • 7 comments

Thanks to the PR https://github.com/rescript-lang/rescript-compiler/pull/6412, we can rename the returned js object key name. But I found little inconsistent with the exisiting @as usage on record.

type t0 = {@as("a") type_: string}

@obj
external make: (@as("b") ~type_: string=?) => t0 = ""

let t0 = { type_: "t0"}

let t1 = make(~type_="t1")

let make = make

generated to

var t0 = {
  a: "t0"
};

var t1 = {
  b: "t1" // shouldn't be a?
};

function make(prim) {
  var tmp = {};
  if (prim !== undefined) {
    tmp.b = prim; // shouldn't be tmp.a = prim?
  }
  return tmp;
}

IMO, @as("b") would make rename the argument to b, but not affecting the returned value that needs to be kept a by @as("a")

What do you think? @DZakh

mununki avatar Oct 17 '23 12:10 mununki

I think it was always an unsafe conversion. So, the conversion right now looks correct to me.

DZakh avatar Oct 17 '23 13:10 DZakh

I'd even vote for deprecating @obj in v12 or something :)

DZakh avatar Oct 17 '23 13:10 DZakh

Oh, I meant not unsafe, but inconsistent.

mununki avatar Oct 17 '23 14:10 mununki

How about seperated it to two cased whether the return type field has @as deriver? If it doesn't have @as in the field of return type then it is renamed by the @as("b") from argument of external function. If it has @as in the field of the returned type then it is renamed to @as("a").

mununki avatar Oct 19 '23 07:10 mununki

https://github.com/zth/rescript-relay/issues/467#issuecomment-1766590001, also it won't work with opaque types

DZakh avatar Oct 19 '23 08:10 DZakh

zth/rescript-relay#467 (comment), also it won't work with opaque types

I think this is an issue to resolve in rescript-relay, not compiler.

mununki avatar Oct 19 '23 11:10 mununki