Skip to content
New issue

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

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

Already on GitHub? # to your account

Default values for function args breaks currying #983

Closed
johnhaley81 opened this issue Dec 12, 2023 · 7 comments
Closed

Default values for function args breaks currying #983

johnhaley81 opened this issue Dec 12, 2023 · 7 comments

Comments

@johnhaley81
Copy link

johnhaley81 commented Dec 12, 2023

I'm trying to call a function being passed to me in a Js.t({..}) and it's not being called as I expected.

ReasonML Code:

"onNavigate": (query, event) => props##applyFilter(. query, event),

Function in JS Code side:

applyFilter: (query, event = {}) => {
  // snip
};

I think what is happening is the second arg event is being assigned a default value so when this function is called with a currying style that arg is not set properly.

So now another interesting thing is, when I call the function in a uncurried way:

"onNavigate": (query, event) => props##applyFilter(. query, event),

I get the following warning:

Alert unused: Unused attribute [@u]

Which I believe is a false positive because in this situation the function is now called correctly!

So it seems that there are 2 main issues:

  1. Default values for function args breaks the currying system (at least when the functions are a property of a JS object)
  2. When calling a function uncurried, if that function is in a Js.t({..}) you'll get a warning about an unused attribute [@u] (playground link)
@johnhaley81 johnhaley81 changed the title Default values for function args Default values for function args breaks currying Dec 12, 2023
@johnhaley81
Copy link
Author

Accidentally submitted the issue too soon, edited it to not just be a scratchpad of me collecting my thoughts :)

@jchavarri
Copy link
Member

@johnhaley81 I think the playground example is lacking type annotations, because the returned type from mel.raw is just 'a which is about anything, so it's missing some key info about the original case you stumbled upon.

I tried to reproduce the problem in #984 but without success. Whenever the type is defined in the Js.t value properly, the generated code will be ok and there won't be any warnings.

What I found is that I'm not able to put the ## method call together with the function application, I will get some strange error about type mismatch with Js__Js_OO.Meth.arity2 🤔

@anmonteiro
Copy link
Member

There are a few things going on here:

Reason -> OCaml translation

I tried to reproduce the problem in #984 but without success. Whenever the type is defined in the Js.t value properly, the generated code will be ok and there won't be any warnings.

this is because you wrote the example in OCaml, and the translation wasn't faithful to the Reason counterpart. This, for example, triggers the alert:

let props  =
  [%mel.raw
    {|
  {
    foo: function(a, b) {
      console.log("a", a);
      console.log("b", b);
    }
  }
|}]

let () = (props##foo 123 "abc" [@u])
(* Here, the alert isn't triggered: *)
(* let () = (props##foo [@u]) 123 "abc" *)

Default arguments

I couldn't find anything special about default arguments in the JS values. I'd appreciate if you could also include a repro here that triggers some kind of bug. For example, this snippet prints the expected values:

let props  =
  [%mel.raw
    {|
  {
    foo: function(a, b="b") {
      console.log("a", a);
      console.log("b", b);
    }
  }
|}]

let () = props##foo 123

output:

a 123
b b

Js__Js_OO.Meth.arity2

  • Your [%mel.raw .. ] expression is inferred as < foo : (int -> string -> unit[@mel.meth]) > Js.t. Melange expects 2 arguments to be applied here instead of 1.
  • We have code to detect this case and print a nicer error message, but I seem to have broken it in some recent PR related to the runtime library wrapping. This is immediately actionable, so I'll provide a fix shortly!

@jchavarri
Copy link
Member

this is because you wrote the example in OCaml, and the translation wasn't faithful to the Reason counterpart. This, for example, triggers the alert

But in this case, as I mentioned above, the type of props is just 'a, which is essentially anything. So I think the warning should be expected.

@anmonteiro
Copy link
Member

But in this case, as I mentioned above, the type of props is just 'a, which is essentially anything. So I think the warning should be expected.

I'm not sure. I think you are correct but I didn't get a chance to investigate that yet!

@anmonteiro
Copy link
Member

But in this case, as I mentioned above, the type of props is just 'a, which is essentially anything. So I think the warning should be expected.

The playground comes very handy to debug this sort of thing. Indeed it's the case that the type inferred by %mel.raw is 'a:

image

It then gets inferred at application time of ## with [@mel.meth]:

image

In this case, [@u] is effectively unnecessary, and the unprocessed alert is correct.

@johnhaley81
Copy link
Author

Sorry for the delayed response. I also looked into this further and it seems that this isn't a bug in Melange, but rather in how the ES6 is being transpiled which is causing the final JS code to somehow break currying in a way that I don't quite understand.

Thanks for looking into it @anmonteiro and @jchavarri!

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

No branches or pull requests

3 participants