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

Fix support for recursive components in JSX4 #733

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
913aa08
add test
mununki Feb 6, 2023
410ee43
fix support for recursive components in JSX4
mununki Feb 6, 2023
9220ef9
add more tests
mununki Feb 6, 2023
92a7736
remove unused rec flag warning
mununki Feb 6, 2023
56bc942
update changelog
mununki Feb 6, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove unused rec flag warning
  • Loading branch information
@mununki
mununki committed Feb 6, 2023
commit 92a77360dd5f1996d7b07f0652b93d74da62bcc8
17 changes: 9 additions & 8 deletions cli/reactjs_jsx_v4.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,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
12 changes: 6 additions & 6 deletions tests/ppx/react/expected/v4.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ module Rec = {

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

@cristianoc cristianoc Feb 6, 2023

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

@cristianoc cristianoc Feb 6, 2023

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

@mununki mununki Feb 6, 2023

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"
Expand All @@ -39,10 +39,10 @@ module Rec1 = {

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

\"V4$Rec1"
Expand All @@ -56,10 +56,10 @@ module Rec2 = {

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

\"V4$Rec2"
Expand Down