-
Notifications
You must be signed in to change notification settings - Fork 464
Ignore unsupplied optional arguments when passing a function as a parameter #6693
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
Comments
Actually in the case of |
The runtime representation of the two functions is different, so it would not be just a type cast. |
+1 I just added Core 1.3.0 in a project and it's very annoying. It can be fixed by using the underscore
but I would love if I had not to explain that to beginners |
I think opt->Option.map(x => Int.toString(x)) will be a clearer workaround for beginners. But yes, this is very unintuitive: // Fine
let x = Int.toString(5)
let x = 5->Int.toString
// Not fine
let x = [5]->Array.map(Int.toString) |
This is going from a function with more parameters to less, but is the opposite any more doable? module Array = {
@send external map: (array<'a>, ('a, ~index: int=?) => 'b) => array<'b> = "map"
}
// Not fine
let x = [5]->Array.map(x => x + 1) |
Good point by @cometkim: This can do weird things in JS: ['1', '2', '3'].map(Number.parseInt)
> [1, NaN, NaN] 😱 |
It could work better in ReScript than in JS. But still maybe not a good idea to encourage this pattern if it is problematic in JS. So I think I can let it go. 🙂 |
This is a feature request.
Example of what I mean (using ReScript 11.1.0-rc.5, uncurried mode, ReScript Core, from my work on rescript-lang/rescript-core#201):
Would it be possible to make 2. compile and work, too?
We will run into such situations more often when we change the standard library to follow this style for bindings (single binding for with optional arguments instead of multiple bindings for a JS function).
I already ran into this in practice with Core's
Int.fromString
which has an optionalradix
argument.In TypeScript, this works fine BTW:
It would be interesting to explore if we can make it work in ReScript, too.
The text was updated successfully, but these errors were encountered: