Improve error message for plain functions used as JSX components

Author: cknitt

compiler/ml/error_message_utils.ml

@@ -107,6 +107,7 @@ type type_clash_context =
       is_constant: string option;
     }
   | FunctionArgument of {optional: bool; name: string option}
+  | JsxComponent
   | BracedIdent
   | Statement of type_clash_statement
   | ForLoopCondition
@@ -127,6 +128,7 @@ let context_to_string = function
   | Some TryReturn -> "TryReturn"
   | Some StringConcat -> "StringConcat"
   | Some (FunctionArgument _) -> "FunctionArgument"
+  | Some JsxComponent -> "JsxComponent"
   | Some ComparisonOperator -> "ComparisonOperator"
   | Some IfReturn -> "IfReturn"
   | Some TernaryReturn -> "TernaryReturn"
@@ -145,6 +147,7 @@ let error_type_text ppf type_clash_context =
     | Some ArrayValue -> "This array item has type:"
     | Some (SetRecordField _) ->
       "You're assigning something to this field that has type:"
+    | Some JsxComponent -> "This JSX tag resolves to:"
     | _ -> "This has type:"
   in
   fprintf ppf "%s" text
@@ -162,6 +165,7 @@ let error_expected_type_text ppf type_clash_context =
     | None -> ());
 
     fprintf ppf " is expecting:"
+  | Some JsxComponent -> fprintf ppf "But JSX component positions require:"
   | Some ComparisonOperator ->
     fprintf ppf "But it's being compared to something of type:"
   | Some SwitchReturn -> fprintf ppf "But this switch is expected to return:"
@@ -396,6 +400,17 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf
       \  - Remove the @{<info>await@} if this is not expected to be a promise\n\
       \  - Wrap the expression in @{<info>Promise.resolve@} to convert the \
        expression to a promise"
+  | Some JsxComponent, _ ->
+    fprintf ppf
+      "\n\n\
+      \  JSX tags must resolve to a React component, not a plain function.\n\n\
+      \  Possible solutions:\n\
+      \  - If this function takes labeled props, annotate it with \
+       @{<info>@react.component@}\n\
+      \  - If this function takes a single props record, annotate it with \
+       @{<info>@react.componentWithProps@}\n\
+      \  - If this is already a valid component-like value, wrap it with \
+       @{<info>React.component(...)@}"
   | Some IfReturn, _ ->
     fprintf ppf
       "\n\n\

compiler/ml/typecore.ml

@@ -2525,7 +2525,10 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
     wrap_trace_gadt_instances env (lower_args env []) ty;
     begin_def ();
     let total_app = not partial in
-    let context = type_clash_context_from_function sexp sfunct in
+    let context =
+      if transformed_jsx then Some JsxComponent
+      else type_clash_context_from_function sexp sfunct
+    in
     let args, ty_res, fully_applied =
       match translate_unified_ops env funct sargs with
       | Some (targs, result_type) -> (targs, result_type, true)
@@ -3925,15 +3928,19 @@ and type_application ~context total_app env funct (sargs : sargs) :
           if (not optional) && is_optional l' then
             Location.prerr_warning sarg0.pexp_loc
               (Warnings.Nonoptional_label (Printtyp.string_of_label l));
+          let argument_context =
+            match (context, args) with
+            | Some JsxComponent, [] -> Some JsxComponent
+            | Some JsxComponent, _ ->
+              type_clash_context_for_function_argument ~label:l' None sarg0
+            | _ ->
+              type_clash_context_for_function_argument ~label:l' context sarg0
+          in
           ( sargs,
             omitted,
             Some
               (if (not optional) || is_optional l' then fun () ->
-                 type_argument
-                   ~context:
-                     (type_clash_context_for_function_argument ~label:l' context
-                        sarg0)
-                   env sarg0 ty ty0
+                 type_argument ~context:argument_context env sarg0 ty ty0
                else fun () ->
                  option_some
                    (type_argument

tests/build_tests/super_errors/expected/jsx_plain_function_component.res.expected

@@ -0,0 +1,19 @@
+
+  We've found a bug for you!
+  /.../fixtures/jsx_plain_function_component.res:26:10-16
+
+  24 │ }
+  25 │ 
+  26 │ let _ = <Wrapper value="hello" />
+  27 │ 
+
+  This JSX tag resolves to: Wrapper.props => Jsx.element
+  But JSX component positions require:
+    React.component<'a> (defined as Jsx.component<'a>)
+
+  JSX tags must resolve to a React component, not a plain function.
+
+  Possible solutions:
+  - If this function takes labeled props, annotate it with @react.component
+  - If this function takes a single props record, annotate it with @react.componentWithProps
+  - If this is already a valid component-like value, wrap it with React.component(...)

tests/build_tests/super_errors/fixtures/jsx_plain_function_component.res

@@ -0,0 +1,26 @@
+module React = {
+  type element = Jsx.element
+  type componentLike<'props, 'return> = Jsx.componentLike<'props, 'return>
+  type component<'props> = Jsx.component<'props>
+
+  external component: componentLike<'props, element> => component<'props> = "%component_identity"
+  external string: string => element = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (component<'props>, 'props) => element = "jsx"
+
+  @module("react/jsx-runtime")
+  external jsxs: (component<'props>, 'props) => element = "jsxs"
+}
+
+module ReactDOM = {
+  external someElement: React.element => option<React.element> = "%identity"
+  @module("react/jsx-runtime")
+  external jsx: (string, JsxDOM.domProps) => Jsx.element = "jsx"
+}
+
+module Wrapper = {
+  type props = {value: string}
+  let make = (props: props) => <div> {React.string(props.value)} </div>
+}
+
+let _ = <Wrapper value="hello" />