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

wrong error message in uncurried mode #6413

Closed
tsnobip opened this issue Sep 23, 2023 · 7 comments · Fixed by #6414
Closed

wrong error message in uncurried mode #6413

tsnobip opened this issue Sep 23, 2023 · 7 comments · Fixed by #6414
Labels

Comments

@tsnobip
Copy link
Contributor

tsnobip commented Sep 23, 2023

I've noticed a bug in the error message that is provided when you are in uncurried mode and you provide a curried function to an uncurried signature:

module Foo: {
  let get: (Js.Dict.t<'a>, Js.Dict.key) => option<'a>
} = {
  let get = Js.Dict.get
}

this yields the following error:

[E] Line 3, column 4:

Signature mismatch:
  ...
  Values do not match:
    let get: (Js.Dict.t<'a>, Js.Dict.key) => option<'a>
  is not included in
    let get: (Js.Dict.t<'a>, Js.Dict.key) => option<'a>
  playground.res:2:3-53: Expected declaration
  playground.res:4:7-9: Actual declaration

Tested in v11.0.0-rc.3 (playground link).

Interestingly enough, it shows the correct error message when inlining the signature:

let get: (Js.Dict.t<'a>, Js.Dict.key) => option<'a> = Js.Dict.get
This function is a curried function where an uncurried function is expected

Originally got suspicious about this bug from this post on the forum.

@tsnobip tsnobip added the bug label Sep 23, 2023
@youngkidwarrior
Copy link

Here is another case of this issue

image

Compiler Error:

This function is a curried function where an uncurried function is expected

@tsnobip
Copy link
Contributor Author

tsnobip commented Sep 26, 2023

This function is a curried function where an uncurried function is expected

Hmm in this case the error message is correct, isn't it?

@youngkidwarrior
Copy link

Yeah I guess for this specific case it'd better to raise an issue in rescript relay for generating an uncurried use function

@tsnobip
Copy link
Contributor Author

tsnobip commented Sep 27, 2023

Yeah I guess for this specific case it'd better to raise an issue in rescript relay for generating an uncurried use function

it's also unclear to me why Js.Dict.get is curried, there might be another related bug.

@cristianoc
Copy link
Collaborator

cristianoc commented Sep 27, 2023

Yeah I guess for this specific case it'd better to raise an issue in rescript relay for generating an uncurried use function

it's also unclear to me why Js.Dict.get is curried, there might be another related bug.

The compiler provides curried and uncurried versions of pervasives.
It does not do that with every library in the compiler.
The idea is to migrate to ReScript Core, which is built in uncurried mode when the project is compiled as uncurried.

@DZakh
Copy link
Contributor

DZakh commented Sep 27, 2023

Related to this #6147

@DZakh
Copy link
Contributor

DZakh commented Sep 27, 2023

I think that we should use the @rescript/std as an automatic dependency for rescript projects instead of getting Js_dict and etc from compiler. This way the uncurried/curried problem will be automatically solved.

Also, it'll be an improvement for packages targeting both rescript and ts only users (eg like rescript-struct). For example, now I have two options to make it work:

  1. Add rescript as a peerDependency. It's fine, but in this case ts only users will need to install the whole rescript compiler, why they need a few runtime modules. It's 153mb which is not the best DX.
  2. Build ts only library version with inlined rescript runtime modules. It'll be duplicated code in the bundle in case of a mixed ts/ReScript codebase.

So it can be solved by making rescript use @rescript/std. So if there's no compiler needed, only @rescript/std can be required as a peerDependency.

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

Successfully merging a pull request may close this issue.

4 participants