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

Support mel.as in mel.obj #834

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Support mel.as in mel.obj #834

merged 3 commits into from
Nov 13, 2023

Conversation

jchavarri
Copy link
Member

This was never supported so when having to introduce different identifiers on JS, we would have to resort back to deriving abstract, e.g. for the DOM props in reason-react.

There would be no need to use deriving abstract if mel.as was supported in mel.obj which is what this PR allows.

@anmonteiro
Copy link
Member

This seems to already have been supported, e.g.:

external ff :
hi:int ->
lo:(_ [@mel.as 3]) ->
lo2:(_ [@mel.as {json|{hi:-3 }|json}]) ->
lo3:(_ [@mel.as -1]) ->
lo4:(_ [@mel.as {json|-3|json}]) ->
_ = "" [@@mel.obj]

There seems to be 2 use cases for @mel.as, and they're a bit at odds with each other (or at least a bit confusing):

1. the current use case, e.g.:

external ff : hi:int -> lo:(_[@mel.as 1]) -> _ = "" [@@mel.obj]

let u = ff ~hi:4

generates:

// Generated by Melange
'use strict';


var u = {
  hi: 4,
  lo: 1
};

exports.u = u;
/* No side effect */

2. your proposed use case

external func :   foo:(string [@mel.as "bar"]) -> ...

generating bar in the object key instead of foo.

Discussion

A few ways I see forward:

  1. distinguish Ptyp_any and don't attempt to rename in that case, falling back to the previous behavior
  2. use a new @mel.rename_label attribute (which I find ugly)
  3. .. more?

@jchavarri
Copy link
Member Author

jchavarri commented Oct 26, 2023

Another option is to change the positioning of the attribute. I chose to annotate the type of the labelled argument because it's imo more ergonomic, but should we annotate the whole arrow type instead? e.g.

external ff :
  ((hi:int -> ((lo:string -> _)[@mel.as "low"]))[@mel.as "high"]) = ""
[@@mel.obj ]

instead of

external ff : hi:(int[@mel.as "high"]) -> lo:(string[@mel.as "low"]) -> _ = ""
[@@mel.obj]

The first option looks way more readable in Reason syntax than in OCaml syntax:

[@mel.obj]
external ff:
  [@mel.as "high"] ((~hi: int) => [@mel.as "low"] ((~lo: string) => _));

@anmonteiro
Copy link
Member

I'd rather keep it on the type, then. even in reason that feels very very weird to write.

@anmonteiro
Copy link
Member

@jchavarri given your recent constraints around React 18, would you like me to finish this PR?

@jchavarri
Copy link
Member Author

Yes, I'd appreciate it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants