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

Fix support for recursive components in JSX4 #733

Merged
merged 5 commits into from
Feb 6, 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 @@ -59,6 +59,7 @@
- Fix issue where error messages related to non-existent props were displayed without location information https://github.com/rescript-lang/syntax/pull/730
- Fix issue where uncurried functions were incorrectly converting the type of a prop given as a default value to curried https://github.com/rescript-lang/syntax/pull/731
- Fix issue with printing async functions with locally abstract types https://github.com/rescript-lang/syntax/pull/732
- Fix support for recursive components in JSX V4 https://github.com/rescript-lang/syntax/pull/733

#### :eyeglasses: Spec Compliance

Expand Down
24 changes: 15 additions & 9 deletions cli/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,12 @@ let transformStructureItem ~config mapper item =
in
let innerExpression =
Exp.apply
(Exp.ident (Location.mknoloc @@ Lident fnName))
(Exp.ident
(Location.mknoloc
@@ Lident
(match recFlag with
| Recursive -> internalFnName
| Nonrecursive -> fnName)))
([(Nolabel, Exp.ident (Location.mknoloc @@ Lident "props"))]
@
match hasForwardRef with
Expand Down Expand Up @@ -1185,14 +1190,15 @@ let transformStructureItem ~config mapper item =
| Recursive ->
( [
bindingWrapper
(Exp.let_ ~loc:emptyLoc Recursive
[
makeNewBinding binding expression internalFnName;
Vb.mk
(Pat.var {loc = emptyLoc; txt = fnName})
fullExpression;
]
(Exp.ident {loc = emptyLoc; txt = Lident fnName}));
(Exp.let_ ~loc:emptyLoc Nonrecursive
[makeNewBinding binding expression internalFnName]
(Exp.let_ ~loc:emptyLoc Nonrecursive
[
Vb.mk
(Pat.var {loc = emptyLoc; txt = fnName})
fullExpression;
]
(Exp.ident {loc = emptyLoc; txt = Lident fnName})));
],
None )
| Nonrecursive ->
Expand Down
52 changes: 52 additions & 0 deletions tests/ppx/react/expected/v4.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,55 @@ module AnotherName = {
\"V4$AnotherName$anotherName"
}
}

module Rec = {
type props = {}

let rec make = {
@merlin.focus
let \"make$Internal" = (_: props) => {
make(({}: props))
}
let make = {
let \"V4$Rec" = props => \"make$Internal"(props)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part seems to be doing noting in the case of recursive functions.
That was true already before, so that's OK. Just, not very useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since recursive components are rare, let's just move on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part seems to be making this expression in js output:

function make(_props) {
  while(true) {
    _props = {};
    continue ;
  };
}

var V4$Rec = { // <- here
  make: make
};


\"V4$Rec"
}
make
}
}

module Rec1 = {
type props = {}

let rec make = {
@merlin.focus
let \"make$Internal" = (_: props) => {
React.null
}
let make = {
let \"V4$Rec1" = props => \"make$Internal"(props)

\"V4$Rec1"
}
make
}
}

module Rec2 = {
type props = {}

let rec make = {
@merlin.focus
let \"make$Internal" = (_: props) => {
mm(({}: props))
}
let make = {
let \"V4$Rec2" = props => \"make$Internal"(props)

\"V4$Rec2"
}
make
}
and mm = x => make(x)
}
22 changes: 22 additions & 0 deletions tests/ppx/react/v4.res
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,25 @@ module AnotherName = {
@react.component
let anotherName = (~x) => React.string(x)
}

module Rec = {
@react.component
let rec make = () => {
make({}:props)
}
}

module Rec1 = {
@react.component
let rec make = () => {
React.null
}
}

module Rec2 = {
@react.component
let rec make = () => {
mm(({}: props))
}
and mm = (x) => make(x)
}