Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@as support for @obj makes inconsistent conversion #6440

Closed
mununki opened this issue Oct 17, 2023 · 7 comments
Closed

@as support for @obj makes inconsistent conversion #6440

mununki opened this issue Oct 17, 2023 · 7 comments
Labels
stale Old issues that went stale

Comments

@mununki
Copy link
Member

mununki commented Oct 17, 2023

Thanks to the PR #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

@DZakh
Copy link
Contributor

DZakh commented Oct 17, 2023

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

@DZakh
Copy link
Contributor

DZakh commented Oct 17, 2023

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

@mununki
Copy link
Member Author

mununki commented Oct 17, 2023

Oh, I meant not unsafe, but inconsistent.

@mununki
Copy link
Member Author

mununki commented Oct 19, 2023

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").

@DZakh
Copy link
Contributor

DZakh commented Oct 19, 2023

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

@mununki
Copy link
Member Author

mununki commented Oct 19, 2023

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.

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Old issues that went stale label Oct 14, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Old issues that went stale
Projects
None yet
Development

No branches or pull requests

2 participants