Newbie experience: Bitten by curry

I am in favour of automatic curry but today, as a newbie, I have been bitten by it :sweat_smile:

Had this code:

  let fileArg = program.args[0]
  let fileToMove = switch fileArg {
  | Some(path) => Node.Path.resolve(path)
  | _ => {
      Js.log(Chalk.red(`No file specified. Please provide a file name to move`))
      Node.Process.exit(1)
    }
  }

Which was compiling to very weird JS with an unneeded function wrap:

  var fileArg = Caml_array.get(program.args, 0);
  var fileToMove = fileArg !== undefined ? (function (param) {
        return Path.resolve(fileArg, param);
      }) : (console.log(Chalk.red("No file specified. Please provide a file name to move")), Process.exit(1));

And I was wondering… why??
Because Path.resolve expects TWO strings (just noticed the function signature). I am not sure why because the node function is variadic and the single string usage is the most common for me, but I guess I am not the majority :grinning_face_with_smiling_eyes:.

Just wanted to share my experience so it may help others in a similar situation (newbies not understanding some weird output)

1 Like

As a side note, I had to write my own binding so I can use the single argument version:

@module("path") external resolve: string => string = "resolve"

Not sure why the built in buildings do not allow this usage and have instead a resolve2 function for the two inputs scenario.

This should probably have been a type error. What are the type signatures of Node.Path.resolve and Node.Process.exit used in the original code? I.e., specifically which binding library are you using?

I don’t blame you but this reminds me of this image (props to my buddy justin).

I know you didn’t write this interface, I’m not criticizing you at all, but this is simply a bad interface. A common problem I see from people using rescript is to dumb it down to the level of javascript and then complain that some types or currying or some other language feature isn’t worth it. (again, not your fault. this is an obvious footgun.)

But, even though this interface sets you up to fail, we can look at the docs (like you did) (or even the history of these docs…) and see it’s actually not a terrible interface in javascript. it just didn’t translate well to rescript.

https://nodejs.org/api/path.html#path_path_resolve_paths

The interface is better translated as:

@module("path") external resolve: array<string> => string = "resolve"

Which would probably (idk?) crash if you passed it an empty array but it at least shows the intent.

And the old docs actually write it even better:

https://nodejs.org/docs/latest-v0.10.x/api/path.html#path_path_resolve_from_to

Which would nicely translate to:

let resolve: (~from: array<string>=?, string) => string;

I haven’t been up on rescript lately, so I don’t know if there’s some ffi magic to get zero-cost binding there (though you’re touching the file system, so a single extra function call is nothing here), or if you’d have to do a little work to get it in that state, but it’d be worth it imo, because you won’t immediately shoot yourself in the foot with it.

5 Likes

I realized that Daniel was using ReScript’s built-in Node.Path.resolve binding: rescript-compiler/node_path.ml at b7e21b4a4f8d75e26302b5c1246b53c8454e64fd · rescript-lang/rescript-compiler · GitHub

And the Node.Process.exit binding here: rescript-compiler/node_process.mli at b7e21b4a4f8d75e26302b5c1246b53c8454e64fd · rescript-lang/rescript-compiler · GitHub

So one branch of the code had a type of string => string, and the other had a type of 'a, so they type-checked against each other and were accepted by the compiler. So yeah, definitely a footgun.

Looks like the Node.Path.resolve binding was written before varargs (splice) support was added, and it was never updated. Reason-nodejs library does it correctly: reason-nodejs/Path.re at b7b41349f85665d6cd7fc76a170229428e7ec94c · sikanhe/reason-nodejs · GitHub

4 Likes

Some interesting comments here.
As I said, I am in favor of typing and currying (that’s why am I using rescript after all), so I’m not blaming the language but myself.
However it is true that the function interface is not consistent with the rest of the modules, so maybe I’m not that stupid :grin:.
Before writing that lines I used Path.join, which interface was indeed array<string>, so in this case I just assumed that if it not was that it was because it was a single argument. In fact I tried first resolve1 because I was expecting resolve to follow the other variadic translations.

The other issues highlighted here are even more complex, so they make me feel even better :grin:

2 Likes

Thinking about this a bit more, the variable fileToMove at this point would have had the type string => string, and you would have gotten a type error as soon as you tried to use it as a string. So I think the compiler has you covered in this case :slight_smile:

Of course, ultimately the binding was not great.