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

Unintuitive error message from PPX: prop '' is not a valid prop for 'foo' #109

Closed
glennsl opened this issue Jan 10, 2022 · 2 comments
Closed

Comments

@glennsl
Copy link
Contributor

glennsl commented Jan 10, 2022

Passing a non-labeled argument to an element constructor/"function" (what do we call these?) results in a rather unintuitive error message.

Minimal repro

[@@@react.dom]

let () = area 51

Actual outcome

File "src/main.ml", line 3, characters 9-16:
3 | let () = area 51
             ^^^^^^^
Error: prop '' isn't a valid prop for a 'area'

Expected outcome

In this case I was really expecting it to call a function I had defined myself, but realizing the actual problem I would expect an error more along the lines of "Unexpected unlabeled argument. Props must be passed to elements as labeled arguments." Perhaps also prefixed with something like "react.dom PPX error: " to identify the source of the error.

And just to add some nitpicking: "a" should become "an" when followed by a vowel (or a syllable that sounds like a vowel), so "...a 'area'" should really be "...an 'area'". But that would be really complicated to implement and probably overkill for this, so maybe just go with "...isn't a valid prop for 'area'"?

@davesnx
Copy link
Member

davesnx commented Jan 11, 2022

Totally. That's a terrible error message, in fact, the error that is triggered have nothing to do with your case. We should raise an error when some nonlabelled argument is being passed and improve the error message of the "is not a valid prop". I tried to describe my thoughts on #94.

Thanks for opening the issue!

@jchavarri
Copy link
Collaborator

Gonna close this as there's no @@@react.dom pre-processing of element tags anymore 🧹

The error now is st like this:

Input:

open React.Dom.Dsl
open Html

let () = area 51

Error:

4 | let () = area 51
                  ^^
Error: This expression has type int but an expression was expected of type
         t array

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

No branches or pull requests

3 participants