Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Port of PR Fix issue with JSX V4 when components are nested https://g… #738

Merged
merged 1 commit into from
Mar 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
- Fix formatting of `switch` expressions that contain braced `cases` inside https://github.com/rescript-lang/syntax/pull/735
- Fix formatting of props spread for multiline JSX expression https://github.com/rescript-lang/syntax/pull/736
- Fix issue with JSX V4 and newtype https://github.com/rescript-lang/syntax/pull/737
- Fix issue with JSX V4 when components are nested https://github.com/rescript-lang/syntax/pull/738

#### :eyeglasses: Spec Compliance

Expand Down
4 changes: 2 additions & 2 deletions cli/reactjs_jsx_ppx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ let getMapper ~config =
| Pstr_attribute attr -> processConfigAttribute attr config
| _ -> ());
let item = default_mapper.structure_item mapper item in
if config.version = 3 then transformStructureItem3 mapper item
else if config.version = 4 then transformStructureItem4 mapper item
if config.version = 3 then transformStructureItem3 item
else if config.version = 4 then transformStructureItem4 item
else [item])
items
|> List.flatten
Expand Down
14 changes: 6 additions & 8 deletions cli/reactjs_jsx_v3.ml
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,7 @@ let jsxMapper ~config =
args
in

let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes =
let expr = mapper.expr mapper expr in
let rec recursivelyTransformNamedArgsForMake expr args newtypes =
match expr.pexp_desc with
(* TODO: make this show up with a loc. *)
| Pexp_fun (Labelled "key", _, _, _) | Pexp_fun (Optional "key", _, _, _) ->
Expand Down Expand Up @@ -494,7 +493,7 @@ let jsxMapper ~config =
| _ -> None
in

recursivelyTransformNamedArgsForMake mapper expression
recursivelyTransformNamedArgsForMake expression
((arg, default, pattern, alias, pattern.ppat_loc, type_) :: args)
newtypes
| Pexp_fun
Expand All @@ -517,10 +516,9 @@ let jsxMapper ~config =
"React: react.component refs only support plain arguments and type \
annotations."
| Pexp_newtype (label, expression) ->
recursivelyTransformNamedArgsForMake mapper expression args
(label :: newtypes)
recursivelyTransformNamedArgsForMake expression args (label :: newtypes)
| Pexp_constraint (expression, _typ) ->
recursivelyTransformNamedArgsForMake mapper expression args newtypes
recursivelyTransformNamedArgsForMake expression args newtypes
| _ -> (args, newtypes, None)
in

Expand Down Expand Up @@ -586,7 +584,7 @@ let jsxMapper ~config =
in

let nestedModules = ref [] in
let transformStructureItem mapper item =
let transformStructureItem item =
match item with
(* external *)
| {
Expand Down Expand Up @@ -825,7 +823,7 @@ let jsxMapper ~config =
let props = getPropsAttr payload in
(* do stuff here! *)
let namedArgList, newtypes, forwardRef =
recursivelyTransformNamedArgsForMake mapper
recursivelyTransformNamedArgsForMake
(modifiedBindingOld binding)
[] []
in
Expand Down
21 changes: 10 additions & 11 deletions cli/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -598,9 +598,7 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs
})
args

let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
=
let expr = mapper.expr mapper expr in
let rec recursivelyTransformNamedArgsForMake expr args newtypes coreType =
match expr.pexp_desc with
(* TODO: make this show up with a loc. *)
| Pexp_fun (Labelled "key", _, _, _) | Pexp_fun (Optional "key", _, _, _) ->
Expand Down Expand Up @@ -647,7 +645,7 @@ let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
| _ -> None
in

recursivelyTransformNamedArgsForMake mapper expression
recursivelyTransformNamedArgsForMake expression
((arg, default, pattern, alias, pattern.ppat_loc, type_) :: args)
newtypes coreType
| Pexp_fun
Expand Down Expand Up @@ -680,10 +678,10 @@ let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
"React: react.component refs only support plain arguments and type \
annotations."
| Pexp_newtype (label, expression) ->
recursivelyTransformNamedArgsForMake mapper expression args
(label :: newtypes) coreType
recursivelyTransformNamedArgsForMake expression args (label :: newtypes)
coreType
| Pexp_constraint (expression, coreType) ->
recursivelyTransformNamedArgsForMake mapper expression args newtypes
recursivelyTransformNamedArgsForMake expression args newtypes
(Some coreType)
| _ -> (args, newtypes, coreType)

Expand Down Expand Up @@ -724,7 +722,7 @@ let check_string_int_attribute_iter =

{Ast_iterator.default_iterator with attribute}

let transformStructureItem ~config mapper item =
let transformStructureItem ~config item =
match item with
(* external *)
| {
Expand Down Expand Up @@ -827,6 +825,8 @@ let transformStructureItem ~config mapper item =
binding with
pvb_pat = {binding.pvb_pat with ppat_loc = emptyLoc};
pvb_loc = emptyLoc;
pvb_attributes =
binding.pvb_attributes |> List.filter otherAttrsPure;
}
in
let fnName = getFnName binding.pvb_pat in
Expand Down Expand Up @@ -870,8 +870,7 @@ let transformStructureItem ~config mapper item =
let modifiedBinding binding =
let hasApplication = ref false in
let wrapExpressionWithBinding expressionFn expression =
Vb.mk ~loc:bindingLoc
~attrs:(List.filter otherAttrsPure binding.pvb_attributes)
Vb.mk ~loc:bindingLoc ~attrs:binding.pvb_attributes
(Pat.var ~loc:bindingPatLoc {loc = bindingPatLoc; txt = fnName})
(expressionFn expression)
in
Expand Down Expand Up @@ -979,7 +978,7 @@ let transformStructureItem ~config mapper item =
in
(* do stuff here! *)
let namedArgList, newtypes, _typeConstraints =
recursivelyTransformNamedArgsForMake mapper
recursivelyTransformNamedArgsForMake
(modifiedBindingOld binding)
[] [] None
in
Expand Down
3 changes: 0 additions & 3 deletions tests/ppx/react/expected/aliasProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
module C0 = {
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

@react.component
let make = (props: props<_, _>) => {
let _ = props.priority
let text = switch props.text {
Expand All @@ -23,7 +22,6 @@ module C0 = {
module C1 = {
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

@react.component
let make = (props: props<_, _>) => {
let p = props.priority
let text = switch props.text {
Expand All @@ -43,7 +41,6 @@ module C1 = {
module C2 = {
type props<'foo> = {foo?: 'foo}

@react.component
let make = (props: props<_>) => {
let bar = switch props.foo {
| Some(foo) => foo
Expand Down
1 change: 0 additions & 1 deletion tests/ppx/react/expected/commentAtTop.res.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
type props<'msg> = {msg: 'msg} // test React JSX file

@react.component
let make = ({msg, _}: props<_>) => {
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
}
Expand Down
2 changes: 0 additions & 2 deletions tests/ppx/react/expected/defaultValueProp.res.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
module C0 = {
type props<'a, 'b> = {a?: 'a, b?: 'b}
@react.component
let make = (props: props<_, _>) => {
let a = switch props.a {
| Some(a) => a
Expand All @@ -22,7 +21,6 @@ module C0 = {
module C1 = {
type props<'a, 'b> = {a?: 'a, b: 'b}

@react.component
let make = (props: props<_, _>) => {
let a = switch props.a {
| Some(a) => a
Expand Down
2 changes: 0 additions & 2 deletions tests/ppx/react/expected/fileLevelConfig.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ module V3 = {
module V4C = {
type props<'msg> = {msg: 'msg}

@react.component
let make = ({msg, _}: props<_>) => {
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
}
Expand All @@ -36,7 +35,6 @@ module V4C = {
module V4A = {
type props<'msg> = {msg: 'msg}

@react.component
let make = ({msg, _}: props<_>) => {
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
}
Expand Down
1 change: 0 additions & 1 deletion tests/ppx/react/expected/firstClassModules.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ module Select = {
items: 'items,
}

@react.component
let make = (
type a key,
{model, selected, onChange, items, _}: props<
Expand Down
4 changes: 0 additions & 4 deletions tests/ppx/react/expected/forwardRef.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ module V4C = {
ref?: 'ref,
}

@react.component
let make = (
{?className, children, _}: props<_, _, ReactRef.currentDomRef>,
ref: Js.Nullable.t<ReactRef.currentDomRef>,
Expand Down Expand Up @@ -103,7 +102,6 @@ module V4C = {
}
type props = {}

@react.component
let make = (_: props) => {
let input = React.useRef(Js.Nullable.null)

Expand Down Expand Up @@ -134,7 +132,6 @@ module V4A = {
ref?: 'ref,
}

@react.component
let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
ReactDOM.jsxs(
"div",
Expand All @@ -160,7 +157,6 @@ module V4A = {
}
type props = {}

@react.component
let make = (_: props) => {
let input = React.useRef(Js.Nullable.null)

Expand Down
4 changes: 2 additions & 2 deletions tests/ppx/react/expected/interface.res.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module A = {
type props<'x> = {x: 'x}
@react.component let make = ({x, _}: props<_>) => React.string(x)
let make = ({x, _}: props<_>) => React.string(x)
let make = {
let \"Interface$A" = (props: props<_>) => make(props)
\"Interface$A"
Expand All @@ -10,7 +10,7 @@ module A = {
module NoProps = {
type props = {}

@react.component let make = (_: props) => ReactDOM.jsx("div", {})
let make = (_: props) => ReactDOM.jsx("div", {})
let make = {
let \"Interface$NoProps" = props => make(props)

Expand Down
1 change: 0 additions & 1 deletion tests/ppx/react/expected/interfaceWithRef.res.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
type props<'x, 'ref> = {x: 'x, ref?: 'ref}
@react.component
let make = (
{x, _}: props<string, ReactDOM.Ref.currentDomRef>,
ref: Js.Nullable.t<ReactDOM.Ref.currentDomRef>,
Expand Down
2 changes: 0 additions & 2 deletions tests/ppx/react/expected/mangleKeyword.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ let c31 = React.createElement(C31.make, C31.makeProps(~_open="x", ()))
module C4C0 = {
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

@react.component
let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
let make = {
let \"MangleKeyword$C4C0" = (props: props<_>) => make(props)
Expand All @@ -44,7 +43,6 @@ let c4c1 = React.createElement(C4C1.make, {_open: "x", _type: "t"})
module C4A0 = {
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}

@react.component
let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
let make = {
let \"MangleKeyword$C4A0" = (props: props<_>) => make(props)
Expand Down
21 changes: 21 additions & 0 deletions tests/ppx/react/expected/nested.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module Outer = {
type props = {}
let make = (_: props) => {
module Inner = {
type props = {}

let make = (_: props) => ReactDOM.jsx("div", {})
let make = {
let \"Nested$Outer" = props => make(props)

\"Nested$Outer"
}
}

React.jsx(Inner.make, {})
}
let make = {
let \"Nested$Outer" = props => make(props)
\"Nested$Outer"
}
}
5 changes: 0 additions & 5 deletions tests/ppx/react/expected/newtype.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ module V3 = {
module V4C = {
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}

@react.component
let make = (type a, {a, b, c, _}: props<a, array<option<[#Foo(a)]>>, 'a>) =>
ReactDOM.createDOMElementVariadic("div", [])
let make = {
Expand All @@ -41,7 +40,6 @@ module V4C = {
module V4A = {
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}

@react.component
let make = (type a, {a, b, c, _}: props<a, array<option<[#Foo(a)]>>, 'a>) =>
ReactDOM.jsx("div", {})
let make = {
Expand All @@ -54,7 +52,6 @@ module V4A = {
module V4A1 = {
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}

@react.component
let make = (type x y, {a, b, c, _}: props<x, array<y>, 'a>) => ReactDOM.jsx("div", {})
let make = {
let \"Newtype$V4A1" = (props: props<_>) => make(props)
Expand All @@ -70,7 +67,6 @@ module type T = {
module V4A2 = {
type props<'foo> = {foo: 'foo}

@react.component
let make = (type a, {foo, _}: props<module(T with type t = a)>) => {
module T = unpack(foo)
ReactDOM.jsx("div", {})
Expand All @@ -85,7 +81,6 @@ module V4A2 = {
module V4A3 = {
type props<'foo> = {foo: 'foo}

@react.component
let make = (type a, {foo, _}: props<_>) => {
module T = unpack(foo: T with type t = a)
foo
Expand Down
6 changes: 2 additions & 4 deletions tests/ppx/react/expected/noPropsWithKey.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module V4CA = {
type props = {}

@react.component let make = (_: props) => ReactDOM.createDOMElementVariadic("div", [])
let make = (_: props) => ReactDOM.createDOMElementVariadic("div", [])
let make = {
let \"NoPropsWithKey$V4CA" = props => make(props)

Expand All @@ -21,7 +21,6 @@ module V4CB = {
module V4C = {
type props = {}

@react.component
let make = (_: props) =>
ReactDOM.createElement(
React.fragment,
Expand All @@ -42,7 +41,7 @@ module V4C = {
module V4CA = {
type props = {}

@react.component let make = (_: props) => ReactDOM.jsx("div", {})
let make = (_: props) => ReactDOM.jsx("div", {})
let make = {
let \"NoPropsWithKey$V4CA" = props => make(props)

Expand All @@ -60,7 +59,6 @@ module V4CB = {
module V4C = {
type props = {}

@react.component
let make = (_: props) =>
React.jsxs(
React.jsxFragment,
Expand Down
1 change: 0 additions & 1 deletion tests/ppx/react/expected/optimizeAutomaticMode.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module User = {
let format = user => "Dr." ++ user.lastName
type props<'doctor> = {doctor: 'doctor}

@react.component
let make = ({doctor, _}: props<_>) => {
ReactDOM.jsx("h1", {id: "h1", children: ?ReactDOM.someElement({React.string(format(doctor))})})
}
Expand Down
Loading