ml-in-barcelona / jsoo-react

js_of_ocaml bindings for ReactJS. Based on ReasonReact.
https://ml-in-barcelona.github.io/jsoo-react
MIT License
138 stars 19 forks source link

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

Closed glennsl closed 2 years ago

glennsl commented 2 years ago

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 commented 2 years ago

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 https://github.com/ml-in-barcelona/jsoo-react/issues/94.

Thanks for opening the issue!

jchavarri commented 2 years ago

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