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.

Sign up for GitHub

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

fix: remove redundant check related to option(_) labelled props #799

Merged
merged 2 commits into from
Oct 22, 2023
Merged

fix: remove redundant check related to option(_) labelled props #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e84cfb
test: demonstrate redundant check in reason-react-ppx
anmonteiro Oct 21, 2023
4721911
fix: remove untrue check related to option(_) labelled props
anmonteiro Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 0 additions & 23 deletions ppx/reason_react_ppx.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,29 +639,6 @@ let jsxMapper =
default,
pattern,
expression ) ->
let () =
match (arg, pattern, default) with
| Optional _, { ppat_desc = Ppat_constraint (_, { ptyp_desc }) }, None -> (
match ptyp_desc with
| Ptyp_constr ({ txt = Lident "option" }, [ _ ]) -> ()
| _ ->
let currentType =
match ptyp_desc with
| Ptyp_constr ({ txt }, []) ->
String.concat "." (Longident.flatten_exn txt)
| Ptyp_constr ({ txt }, _innerTypeArgs) ->
String.concat "." (Longident.flatten_exn txt) ^ "(...)"
| _ -> "..."
in
Ocaml_common.Location.prerr_warning pattern.ppat_loc
(Preprocessor
(Printf.sprintf
"reason-react-ppx: optional argument annotations \
must have an explicit `option` type. Did you mean \
`option(%s)=?`?"
currentType)))
| _ -> ()
in
let alias =
match pattern with
| { ppat_desc = Ppat_alias (_, { txt }) | Ppat_var { txt } } -> txt
Expand Down
30 changes: 30 additions & 0 deletions ppx/test/optional-arg-check.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
Show the error message when an optionally labelled argument has the wrong type

$ cat > dune-project <<EOF
> (lang dune 3.8)
> (using melange 0.1)
> EOF

$ cat > dune <<EOF
> (melange.emit
> (target output)
> (alias mel)
> (emit_stdlib false)
> (libraries reason-react)
> (preprocess (pps melange.ppx reason-react-ppx)))
> EOF

$ cat > x.re <<EOF
> module App = {
> [@react.component]
> let make = (~myProp: bool=?) => React.null;
> };
> EOF

$ dune build @mel
File "x.re", line 3, characters 15-27:
3 | let make = (~myProp: bool=?) => React.null;
^^^^^^^^^^^^
Error: This pattern matches values of type bool
but a pattern was expected which matches values of type 'a option
[1]
Loading