Bug: JSX 4 doesn't work with prop having var with the same name as default value

[DZakh]

https://rescript-lang.org/try?version=v11.0.0-beta.4&code=ALBWGcA8GEHsDsBmBLA5gCgN4DcCmAncZBALgAIAWAGjIFtYATXcgIgEMBXAF1lra+QBjFgF8AlACgJ9BhwA2uMgHVY+ANbgyAXjKYJZMgq5kARtrItEsWC31lg+XG0FcAdIN4AHBLnhc7RnRsaoo66AB+bFomYtoAfGQASk4uruBc+MjwGGySIlIy8ooAIrC44H4q6uZ6BoFs5pbWtgYOKW4etN7wvv51uMZ8IeYRUbnxSe1pGVk5eRJAA

[mununki]

Good catch! We can see why by looking at the code that JSX PPX converted.

module DoesntWork = {
  let a = "foo" // 1
  type props<'a> = {a?: 'a}

  let make = ({?a, _}: props<_>) => { // 2
    let a = switch a {
    | Some(a) => a
    | None => a // <- this should refer to the 1, not 2
    }

    React.string(a)
  }
  ...
}

[mununki]

Maybe we can fix this using alias the props name:

module DoesntWork = {
  let a = "foo"
  type props<'a> = {a?: 'a}

  let make = ({a: ?a_, _}: props<_>) => { // alias a as a_
    let a = switch a_ { // refer a_ here
    | Some(a) => a
    | None => a
    }

    React.string(a)
  }
  ...
}

[mununki]

What do you think? @cristianoc

[DZakh]

I find this more reliable:

module Works = {
  let a = "foo"
  type props<'a> = {a?: 'a}

  let make = (props: props<_>) => {
    let a = switch props {
    | {a} => a
    | _ => a
    }

    React.string(a)
  }
}

It won't break if there's a prop with a_ name. Even though there's a vary little chance for this.

[mununki]

I find this more reliable:

module Works = {
  let a = "foo"
  type props<'a> = {a?: 'a}

  let make = (props: props<_>) => {
    let a = switch props {
    | {a} => a
    | _ => a
    }

    React.string(a)
  }
}

It won't break if there's a prop with a_ name. Even though there's a vary little chance for this.

Wow, I like it. I guess this could also apply to #6375.

[mununki]

If we have other props besides a, it is probably better off destructuring like it was used to.

[DZakh]

How about:

module Works = {
  let a = "foo"
  type props<'a, 'anotherProp1, 'anotherProp2> = {a?: 'a, anotherProp1: 'anotherProp1, anotherProp2: 'anotherProp2}

  let make = (props: props<_, _, _>) => {
    let a = switch props {
    | {a} => a
    | _ => a
    }
    let {anotherProp1, anotherProp2} = props

    React.string(a)
  }
}

[mununki]

How about:

module Works = {
  let a = "foo"
  type props<'a, 'otherProp1, 'otherProp2> = {a?: 'a, otherProp1: 'otherProp1, otherProp2: 'otherProp2}

  let make = (props: props<_, _, _>) => {
    let a = switch props {
    | {a} => a
    | _ => a
    }
    let {otherProp1, otherProp2} = props

    React.string(a)
  }
}

I still prefer this: #6374 (comment)

[DZakh]

What I like in this case #6374 (comment) that it also allows to use the props object directly, which might be useful sometimes.

[mununki]

Actually, there is no props in res expession. Exposing a variable called props to the returned element function body may have other side effects.

[mununki]

There is no props in user land.

module DoesntWork = {
  let a = "foo"
  @react.component
  let make = (~a=a) => React.string(a)
}

[DZakh]

I think props is cleaner than a_ and might even be presented as a feature.
Also, I like that in the generated code the argument now called props instead of param: https://rescript-lang.org/try?version=v10.1.2&code=LYewJgrgNgpgBAdRAJwNYGc4F44G8BQccsALnAIbZwBEAZiCNYXCQJ4AO87yI76APAHJyAGjjCAdiBIALGMgAKPdgEYxk6XMXKATAD4qBIkXIB+AFzjRzE1NnylvFZY33tTkTYp2tj9jpdyHwddTyIAX3xmUjhgclR4HAAKbl50S1S+fgB9MVy4bL0ASmwDI2MYyhx0AHcASxIAYxk4TMxyogAfPHJw0oovbuz+8i9I42IYMlwgzRCPbzn3fz6cNqiJgCl0ADooEABzJNm3PxUir229w+Pg5Z0LrwAlGHJGkh30EmQ6iSPyC4RfCRfCgSCwRAoDA6QzRKYUKh0BhMIhsTitZQCYTqE6+ZRqKx3Pz6WETMyBMLGXHzVSBIn4ym2JbEunM0LMEFEGJxBJUJIzSymcj5anLAmi4l5cIZTE5Yr9DpwAD0SooUDq5EwlE1FGyXkqVFqDWaurwXhVcGQMFo8lNWhggzgAGUQMAYMcSlgDKMJt0AHIgCSJb1jDbGK77f70pyA8O7SO3Nm8B5hogvN4fL4-P4ejnA-BAA

[mununki]

Exposing a variable called props to the returned element function body may have other side effects.

I don't think this is a topic of whether the code is clean or not. What do you think about variables called props being exposed in the function body that user didn't express?

[DZakh]

What do you think about variables called props being exposed in the function body that user didn't express?

I find it fine and that the behaviour can be expected. The same as the props type is available even though it wasn't defined.