Fix support for recursive components in JSX V4.

[cristianoc]

Fixes #5981

[cristianoc]

@mununki feel free to take over from here. (compiler 10.1 etc).

[mununki]

Oh! Thanks!
I couldn't grab a time to look into issues so far, I'll grab time to fix them soon!

[mununki]

I found one thing missing. The rec keyword from the make function of component needs to be removed, because the internal fnName is called inside after transformation. It can't pass the type checker with unused rec flag 8ad9e03

What do you think?

[mununki]

Wondering why the test failed. I ran node scripts/ciTest.js -all on my machine, it passes the test.

[cristianoc]

I found one thing missing. The rec keyword from the make function of component needs to be removed, because the internal fnName is called inside after transformation. It can't pass the type checker with unused rec flag 8ad9e03

What do you think?

That is breakage, not a fix.

Consider the test:

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

You can't remove the toplevel rec as make and mm call each other.

There is some little annoyance, already present in V3, that if you only have make calling itself, you get a warning of unused rec. That aspect could be investigated separately, and might require thinking about a different transformation.

[mununki]

There is some little annoyance, already present in V3, that if you only have make calling itself, you get a warning of unused rec. That aspect could be investigated separately, and might require thinking about a different transformation.

That is correct. It is not applicable except the example calling make itself.

[cristianoc]

There is some little annoyance, already present in V3, that if you only have make calling itself, you get a warning of unused rec. That aspect could be investigated separately, and might require thinking about a different transformation.

That is correct. It is not applicable except the example calling make itself.

Perhaps the warning can be addressed by turning this:

  let rec make = {
     let rec \"make$Internal" = (_: props) => make(({}: props))
     }
     and make = {
       let \"V4$Rec" = props => \"make$Internal"(props)
       \"V4$Rec"
     }
     make
   }
 }

into this (remove rec from make$Internal):

  let rec make = {
     let \"make$Internal" = (_: props) => make(({}: props))
     let make = {
       let \"V4$Rec" = props => \"make$Internal"(props)
       \"V4$Rec"
     }
     make
   }
 }

[mununki]

Perhaps the warning can be addressed by turning this:

  let rec make = {
     let rec \"make$Internal" = (_: props) => make(({}: props))
     }
     and make = {
       let \"V4$Rec" = props => \"make$Internal"(props)
       \"V4$Rec"
     }
     make
   }
 }

into this (remove rec from make$Internal):

  let rec make = {
     let \"make$Internal" = (_: props) => make(({}: props))
     let make = {
       let \"V4$Rec" = props => \"make$Internal"(props)
       \"V4$Rec"
     }
     make
   }
 }

The unused ref flag is not away in the example from the issue:

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
  }
}

[mununki]

Oh, that is correct warning

[cristianoc]

into this (remove rec from make$Internal):

  let rec make = {
     let \"make$Internal" = (_: props) => make(({}: props))
     let make = {
       let \"V4$Rec" = props => \"make$Internal"(props)
       \"V4$Rec"
     }
     make
   }
 }

I think that's the desired transform, which gives a warning only when it is a correct one.

[mununki]

How does it look? rescript-lang/syntax@92a7736

[cristianoc]

How does it look? rescript-lang/syntax@92a7736

Looking good