From 91c17b3d71346e0ce0c0940e7f9b2af4793fda32 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 24 Jul 2025 23:19:22 +0200 Subject: [PATCH 1/4] suggest using jsx fragment where appropriate --- compiler/ml/error_message_utils.ml | 21 +++++++++++++++---- .../jsx_maybe_missing_fragment.res.expected | 16 ++++++++++++++ .../fixtures/jsx_maybe_missing_fragment.res | 20 ++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/jsx_maybe_missing_fragment.res diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 46be9e727f..1701f6ea2d 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -353,11 +353,24 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf -> fprintf ppf "@,@,Dicts are written like: @{dict{\"a\": 1, \"b\": 2}@}@," - | _, Some ({Types.desc = Tconstr (_p1, _, _)}, {desc = Tconstr (p2, _, _)}) + | ( _, + Some + (({Types.desc = Tconstr (_p1, _, _)} as ty), {desc = Tconstr (p2, _, _)}) + ) when Path.same Predef.path_unit p2 -> + fprintf ppf "\n\n"; + let is_jsx_element = + match Ctype.expand_head env ty with + | {desc = Tconstr (Pdot (Pident {name = "Jsx"}, "element", _), _, _)} -> + true + | _ -> false + in + if is_jsx_element then + fprintf ppf + " - Did you forget to wrap this + adjacent JSX in a JSX fragment \ + (@{<>@})?\n"; fprintf ppf - "\n\n\ - \ - Did you mean to assign this to a variable?\n\ + " - Did you mean to assign this to a variable?\n\ \ - If you don't care about the result of this expression, you can \ assign it to @{_@} via @{let _ = ...@} or pipe it to \ @{ignore@} via @{expression->ignore@}\n\n" @@ -714,7 +727,7 @@ let type_clash_context_maybe_option ty_expected ty_res = let type_clash_context_in_statement sexp = match sexp.Parsetree.pexp_desc with - | Pexp_apply _ -> Some (Statement FunctionCall) + | Pexp_apply {transformed_jsx = false} -> Some (Statement FunctionCall) | _ -> None let print_contextual_unification_error ppf t1 t2 = diff --git a/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected b/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected new file mode 100644 index 0000000000..ef423f37ec --- /dev/null +++ b/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected @@ -0,0 +1,16 @@ + + We've found a bug for you! + /.../fixtures/jsx_maybe_missing_fragment.res:18:3-8 + + 16 │ + 17 │ let x = { + 18 │ <>  + 19 │ <> + 20 │ } + + This has type: React.element (defined as Jsx.element) + But it's expected to have type: unit + + - Did you forget to wrap this + adjacent JSX in a JSX fragment (<>)? + - Did you mean to assign this to a variable? + - If you don't care about the result of this expression, you can assign it to _ via let _ = ... or pipe it to ignore via expression->ignore \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/jsx_maybe_missing_fragment.res b/tests/build_tests/super_errors/fixtures/jsx_maybe_missing_fragment.res new file mode 100644 index 0000000000..b0c101a4f0 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/jsx_maybe_missing_fragment.res @@ -0,0 +1,20 @@ +@@config({ + flags: ["-bs-jsx", "4"], +}) + +module React = { + type element = Jsx.element + type componentLike<'props, 'return> = 'props => 'return + type component<'props> = Jsx.component<'props> + + @module("react/jsx-runtime") + external jsx: (component<'props>, 'props) => element = "jsx" + + type fragmentProps = {children?: element} + @module("react/jsx-runtime") external jsxFragment: component = "Fragment" +} + +let x = { + <> + <> +} From 297ed61115f6a3af9f01899649b271cd33457685 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 24 Jul 2025 23:21:16 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b76174e1cc..5f1abd4f4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ - Suggest related functions with the expected arity in errors when it makes sense. https://github.com/rescript-lang/rescript/pull/7712 - Improve error when a constructor expects an inline record. https://github.com/rescript-lang/rescript/pull/7713 - Remove `@meth` attribute. https://github.com/rescript-lang/rescript/pull/7684 +- Apply heuristic to suggest using JSX fragments where we guess that might be what the user wanted. https://github.com/rescript-lang/rescript/pull/7714 #### :house: Internal @@ -50,7 +51,6 @@ - Remove all leftovers of `pinned-dependencies` handling. https://github.com/rescript-lang/rescript/pull/7686 - Add `rust-version` field to Rewatch's `Cargo.toml`. https://github.com/rescript-lang/rescript/pull/7701 - # 12.0.0-beta.2 #### :boom: Breaking Change From 49baf5f87647152dcb379b881641293d43793018 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 25 Jul 2025 18:29:54 +0200 Subject: [PATCH 3/4] update message --- compiler/ml/error_message_utils.ml | 14 ++++++++------ .../jsx_maybe_missing_fragment.res.expected | 3 +-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/ml/error_message_utils.ml b/compiler/ml/error_message_utils.ml index 1701f6ea2d..1e969887f0 100644 --- a/compiler/ml/error_message_utils.ml +++ b/compiler/ml/error_message_utils.ml @@ -368,12 +368,14 @@ let print_extra_type_clash_help ~extract_concrete_typedecl ~env loc ppf if is_jsx_element then fprintf ppf " - Did you forget to wrap this + adjacent JSX in a JSX fragment \ - (@{<>@})?\n"; - fprintf ppf - " - Did you mean to assign this to a variable?\n\ - \ - If you don't care about the result of this expression, you can \ - assign it to @{_@} via @{let _ = ...@} or pipe it to \ - @{ignore@} via @{expression->ignore@}\n\n" + (@{<>@})?\n\ + \ - Did you mean to assign this to a variable?\n\n" + else + fprintf ppf + " - Did you mean to assign this to a variable?\n\ + \ - If you don't care about the result of this expression, you can \ + assign it to @{_@} via @{let _ = ...@} or pipe it to \ + @{ignore@} via @{expression->ignore@}\n\n" | _, Some ({desc = Tobject _}, ({Types.desc = Tconstr _} as t1)) when is_record_type ~extract_concrete_typedecl ~env t1 -> fprintf ppf diff --git a/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected b/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected index ef423f37ec..a21dfbd5b9 100644 --- a/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected +++ b/tests/build_tests/super_errors/expected/jsx_maybe_missing_fragment.res.expected @@ -12,5 +12,4 @@ But it's expected to have type: unit - Did you forget to wrap this + adjacent JSX in a JSX fragment (<>)? - - Did you mean to assign this to a variable? - - If you don't care about the result of this expression, you can assign it to _ via let _ = ... or pipe it to ignore via expression->ignore \ No newline at end of file + - Did you mean to assign this to a variable? \ No newline at end of file From 60cc1e76ab2135e978e6ab5f8ee4f59a19714626 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Fri, 25 Jul 2025 20:17:13 +0200 Subject: [PATCH 4/4] move changelog to right place --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f1abd4f4e..4e6d03896f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ # 12.0.0-beta.4 (Unreleased) +#### :nail_care: Polish + +- Apply heuristic to suggest using JSX fragments where we guess that might be what the user wanted. https://github.com/rescript-lang/rescript/pull/7714 + # 12.0.0-beta.3 #### :boom: Breaking Change @@ -41,7 +45,6 @@ - Suggest related functions with the expected arity in errors when it makes sense. https://github.com/rescript-lang/rescript/pull/7712 - Improve error when a constructor expects an inline record. https://github.com/rescript-lang/rescript/pull/7713 - Remove `@meth` attribute. https://github.com/rescript-lang/rescript/pull/7684 -- Apply heuristic to suggest using JSX fragments where we guess that might be what the user wanted. https://github.com/rescript-lang/rescript/pull/7714 #### :house: Internal