From 05c1cc186bae3d125de6ef7db9769e68fdca993b Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 30 May 2024 15:36:20 +0200 Subject: [PATCH 1/2] PPX v4: mark props type in externals as `@live`. Fixes https://github.com/rescript-lang/rescript-vscode/issues/994 Dead code elimination in the editor tooling complains about props never being read in the `props` type record defined by the V4 ppx. This is because externals don't provide the implementation of the component, and it's in the implementation that props could be read. This OR marks the `props` type definition as `@live` for external components, so the dead code analysis does not fire. --- CHANGELOG.md | 1 + jscomp/syntax/src/jsx_v4.ml | 33 +++++++++++-------- .../expected/externalWithCustomName.res.txt | 2 ++ .../react/expected/externalWithRef.res.txt | 13 ++++++++ .../externalWithTypeVariables.res.txt | 2 ++ .../react/expected/firstClassModules.res.txt | 1 + .../ppx/react/expected/mangleKeyword.res.txt | 2 ++ .../ppx/react/expected/noPropsWithKey.res.txt | 2 ++ .../tests/ppx/react/expected/v4.res.txt | 2 ++ .../tests/ppx/react/externalWithRef.res | 8 +++++ 10 files changed, 52 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0d2392669..7bb1550e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ - Fix issue of incorrect switch cases with identical bodies when mixing object and array. https://github.com/rescript-lang/rescript-compiler/pull/6792 - Fix formatter eats comments on the first argument of an uncurried function. https://github.com/rescript-lang/rescript-compiler/pull/6763 - Fix formatter removes parens in pipe operator with anonymous uncurried function. https://github.com/rescript-lang/rescript-compiler/pull/6766 +- PPX v4: mark props type in externals as `@live` to avoid dead code warnings for prop fields in the editor tooling. https://github.com/rescript-lang/rescript-compiler/pull/6796 #### :house: Internal diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index ad5a99d4f2..29e6e8bfce 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -327,7 +327,7 @@ let make_label_decls named_type_list = Type.field ~loc ~attrs {txt = label; loc} (Typ.var @@ safe_type_from_value @@ Labelled label)) -let make_type_decls props_name loc named_type_list = +let make_type_decls ~attrs props_name loc named_type_list = let label_decl_list = make_label_decls named_type_list in (* 'id, 'className, ... *) let params = @@ -335,7 +335,7 @@ let make_type_decls props_name loc named_type_list = |> List.map (fun core_type -> (core_type, Invariant)) in [ - Type.mk ~loc ~params {txt = props_name; loc} + Type.mk ~attrs ~loc ~params {txt = props_name; loc} ~kind:(Ptype_record label_decl_list); ] @@ -346,22 +346,26 @@ let make_type_decls_with_core_type props_name loc core_type typ_vars = ~manifest:core_type; ] +let live_attr = ({txt = "live"; loc = Location.none}, PStr []) + (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *) -let make_props_record_type ~core_type_of_attr ~typ_vars_of_core_type props_name - loc named_type_list = +let make_props_record_type ~core_type_of_attr ~external_ ~typ_vars_of_core_type + props_name loc named_type_list = + let attrs = if external_ then [live_attr] else [] in Str.type_ Nonrecursive (match core_type_of_attr with - | None -> make_type_decls props_name loc named_type_list + | None -> make_type_decls ~attrs props_name loc named_type_list | Some core_type -> make_type_decls_with_core_type props_name loc core_type typ_vars_of_core_type) (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *) -let make_props_record_type_sig ~core_type_of_attr ~typ_vars_of_core_type - props_name loc named_type_list = +let make_props_record_type_sig ~core_type_of_attr ~external_ + ~typ_vars_of_core_type props_name loc named_type_list = + let attrs = if external_ then [live_attr] else [] in Sig.type_ Nonrecursive (match core_type_of_attr with - | None -> make_type_decls props_name loc named_type_list + | None -> make_type_decls ~attrs props_name loc named_type_list | Some core_type -> make_type_decls_with_core_type props_name loc core_type typ_vars_of_core_type) @@ -950,8 +954,8 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding = let named_type_list = List.fold_left arg_to_type [] named_arg_list in (* type props = { ... } *) let props_record_type = - make_props_record_type ~core_type_of_attr ~typ_vars_of_core_type "props" - pstr_loc named_type_list + make_props_record_type ~core_type_of_attr ~external_:false + ~typ_vars_of_core_type "props" pstr_loc named_type_list in let inner_expression = Exp.apply @@ -1211,8 +1215,8 @@ let transform_structure_item ~config item = in (* type props<'x, 'y> = { x: 'x, y?: 'y, ... } *) let props_record_type = - make_props_record_type ~core_type_of_attr ~typ_vars_of_core_type "props" - pstr_loc named_type_list + make_props_record_type ~core_type_of_attr ~external_:true + ~typ_vars_of_core_type "props" pstr_loc named_type_list in (* can't be an arrow because it will defensively uncurry *) let new_external_type = @@ -1317,9 +1321,10 @@ let transform_signature_item ~config item = | [] -> [] | _ -> [Typ.any ()])) in + let external_ = psig_desc.pval_prim <> [] in let props_record_type = - make_props_record_type_sig ~core_type_of_attr ~typ_vars_of_core_type - "props" psig_loc named_type_list + make_props_record_type_sig ~core_type_of_attr ~external_ + ~typ_vars_of_core_type "props" psig_loc named_type_list in (* can't be an arrow because it will defensively uncurry *) let new_external_type = diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt index 335c670fe5..be73710b13 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithCustomName.res.txt @@ -13,6 +13,7 @@ let t = React.createElement(Foo.component, Foo.componentProps(~a=1, ~b={"1"}, () @@jsxConfig({version: 4, mode: "classic"}) module Foo = { + @live type props<'a, 'b> = {a: 'a, b: 'b} @module("Foo") @@ -24,6 +25,7 @@ let t = React.createElement(Foo.component, {a: 1, b: "1"}) @@jsxConfig({version: 4, mode: "automatic"}) module Foo = { + @live type props<'a, 'b> = {a: 'a, b: 'b} @module("Foo") diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt index dd64f13c65..036b7cf555 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithRef.res.txt @@ -18,6 +18,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @live type props<'x, 'ref> = { x: 'x, ref?: 'ref, @@ -28,9 +29,21 @@ module V4C = { "component" } +module type V4CType = { + @live + type props<'x, 'y> = { + x: 'x, + y: 'y, + } + + @module("someModule") + external make: React.componentLike, React.element> = "component" +} + @@jsxConfig({version: 4, mode: "automatic"}) module V4C = { + @live type props<'x, 'ref> = { x: 'x, ref?: 'ref, diff --git a/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt b/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt index 6850bdde5f..1a62c59653 100644 --- a/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/externalWithTypeVariables.res.txt @@ -16,6 +16,7 @@ module V3 = { @@jsxConfig({version: 4, mode: "classic"}) module V4C = { + @live type props<'x, 'children> = { x: 'x, children: 'children, @@ -28,6 +29,7 @@ module V4C = { @@jsxConfig({version: 4, mode: "automatic"}) module V4C = { + @live type props<'x, 'children> = { x: 'x, children: 'children, diff --git a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt index dd14bf349f..d3dd016e74 100644 --- a/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/firstClassModules.res.txt @@ -88,6 +88,7 @@ module External = { type key type t } + @live type props<'model, 'selected, 'onChange, 'items> = { model: 'model, selected: 'selected, diff --git a/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt b/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt index 087f649aed..d803631282 100644 --- a/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/mangleKeyword.res.txt @@ -47,6 +47,7 @@ module C4C0 = { } } module C4C1 = { + @live type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} external make: @as("open") React.componentLike, React.element> = "default" @@ -68,6 +69,7 @@ module C4A0 = { } } module C4A1 = { + @live type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type} external make: @as("open") React.componentLike, React.element> = "default" diff --git a/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt b/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt index c541cb3e99..90b8603465 100644 --- a/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt @@ -12,6 +12,7 @@ module V4CA = { } module V4CB = { + @live type props = {} @module("c") @@ -51,6 +52,7 @@ module V4CA = { } module V4CB = { + @live type props = {} @module("c") diff --git a/jscomp/syntax/tests/ppx/react/expected/v4.res.txt b/jscomp/syntax/tests/ppx/react/expected/v4.res.txt index 0a548c9021..b0f8ded29c 100644 --- a/jscomp/syntax/tests/ppx/react/expected/v4.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/v4.res.txt @@ -35,12 +35,14 @@ module type TUncurried = { } module E = { + @live type props<'x> = {x: 'x} external make: React.componentLike, React.element> = "default" } module EUncurried = { + @live type props<'x> = {x: 'x} external make: React.componentLike, React.element> = "default" diff --git a/jscomp/syntax/tests/ppx/react/externalWithRef.res b/jscomp/syntax/tests/ppx/react/externalWithRef.res index 83d1512730..9fa0c3b012 100644 --- a/jscomp/syntax/tests/ppx/react/externalWithRef.res +++ b/jscomp/syntax/tests/ppx/react/externalWithRef.res @@ -18,6 +18,14 @@ module V4C = { ) => React.element = "component" } +module type V4CType = { + @module("someModule") @react.component + external make: ( + ~x: string, + ~y: string, + ) => React.element = "component" +} + @@jsxConfig({version: 4, mode: "automatic"}) module V4C = { From f99f2c8338a795a5b8fe536eac7b35bce00924c5 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 31 May 2024 14:51:30 +0200 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5feb06ff24..1574a11ecb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,9 +28,6 @@ #### :bug: Bug Fix - Fix unhandled cases for exotic idents (allow to use exotic PascalCased identifiers for types). https://github.com/rescript-lang/rescript-compiler/pull/6777 -- Fix issue of incorrect switch cases with identical bodies when mixing object and array. https://github.com/rescript-lang/rescript-compiler/pull/6792 -- Fix formatter eats comments on the first argument of an uncurried function. https://github.com/rescript-lang/rescript-compiler/pull/6763 -- Fix formatter removes parens in pipe operator with anonymous uncurried function. https://github.com/rescript-lang/rescript-compiler/pull/6766 - PPX v4: mark props type in externals as `@live` to avoid dead code warnings for prop fields in the editor tooling. https://github.com/rescript-lang/rescript-compiler/pull/6796 #### :house: Internal