Skip to content

Fix JSX V4 build error when component props have the default value with same name #6377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 30, 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

# 11.0.0-rc.3 (Unreleased)

#### :bug: Bug Fix

- Fix issue with JSX V4 when component props have the default value with same name. https://github.com/rescript-lang/rescript-compiler/pull/6377

# 11.0.0-rc.2

#### :rocket: New Feature
Expand Down
32 changes: 28 additions & 4 deletions jscomp/syntax/src/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,13 @@ let rec recursivelyTransformNamedArgsForMake expr args newtypes coreType =
in
let alias =
match pattern with
| {ppat_desc = Ppat_alias (_, {txt}) | Ppat_var {txt}} -> txt
| {
ppat_desc =
( Ppat_alias (_, {txt})
| Ppat_var {txt}
| Ppat_constraint ({ppat_desc = Ppat_var {txt}}, _) );
} ->
txt
| {ppat_desc = Ppat_any} -> "_"
| _ -> getLabel arg
in
Expand Down Expand Up @@ -852,7 +858,7 @@ let vbMatch ~expr (name, default, _, alias, loc, _) =
Vb.mk
(Pat.var (Location.mkloc alias loc))
(Exp.match_
(Exp.ident {txt = Lident alias; loc = Location.none})
(Exp.ident {txt = Lident ("__" ^ alias); loc = Location.none})
[
Exp.case
(Pat.construct
Expand Down Expand Up @@ -978,6 +984,14 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
stripConstraintUnpack ~label pattern
| _ -> pattern
in
let safePatternLabel pattern =
match pattern with
| {ppat_desc = Ppat_var {txt; loc}} ->
{pattern with ppat_desc = Ppat_var {txt = "__" ^ txt; loc}}
| {ppat_desc = Ppat_alias (p, {txt; loc})} ->
{pattern with ppat_desc = Ppat_alias (p, {txt = "__" ^ txt; loc})}
| _ -> pattern
in
let rec returnedExpression patternsWithLabel patternsWithNolabel
({pexp_desc} as expr) =
match pexp_desc with
Expand All @@ -991,16 +1005,26 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
{ppat_desc = Ppat_construct ({txt = Lident "()"}, _)},
expr ) ->
(patternsWithLabel, patternsWithNolabel, expr)
| Pexp_fun (arg_label, _default, ({ppat_loc; ppat_desc} as pattern), expr)
| Pexp_fun (arg_label, default, ({ppat_loc; ppat_desc} as pattern), expr)
-> (
let patternWithoutConstraint =
stripConstraintUnpack ~label:(getLabel arg_label) pattern
in
(*
If prop has the default value as Ident, it will get a build error
when the referenced Ident value and the prop have the same name.
So we add a "__" to label to resolve the build error.
*)
let patternWithSafeLabel =
match default with
| Some _ -> safePatternLabel patternWithoutConstraint
| _ -> patternWithoutConstraint
in
if isLabelled arg_label || isOptional arg_label then
returnedExpression
(( {loc = ppat_loc; txt = Lident (getLabel arg_label)},
{
patternWithoutConstraint with
patternWithSafeLabel with
ppat_attributes =
(if isOptional arg_label then optionalAttrs else [])
@ pattern.ppat_attributes;
Expand Down
13 changes: 13 additions & 0 deletions jscomp/syntax/tests/ppx/react/defaultValueProp.res
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,16 @@ module C1 = {
@react.component
let make = (~a=2, ~b) => React.int(a + b)
}

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

module C3 = {
@react.component
let make = (~disabled as everythingDisabled: bool=false) => {
React.string(everythingDisabled ? "true" : "false")
}
}
26 changes: 13 additions & 13 deletions jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
module C0 = {
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

let make = ({priority: _, ?text, _}: props<_, _>) => {
let text = switch text {
let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
let text = switch __text {
| Some(text) => text
| None => "Test"
}
Expand All @@ -21,8 +21,8 @@ module C0 = {
module C1 = {
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

let make = ({priority: p, ?text, _}: props<_, _>) => {
let text = switch text {
let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
let text = switch __text {
| Some(text) => text
| None => "Test"
}
Expand All @@ -39,8 +39,8 @@ module C1 = {
module C2 = {
type props<'foo> = {foo?: 'foo}

let make = ({foo: ?bar, _}: props<_>) => {
let bar = switch bar {
let make = ({foo: ?__bar, _}: props<_>) => {
let bar = switch __bar {
| Some(foo) => foo
| None => ""
}
Expand All @@ -57,12 +57,12 @@ module C2 = {
module C3 = {
type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}

let make = ({foo: ?bar, ?a, b, _}: props<_, _, _>) => {
let bar = switch bar {
let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
let bar = switch __bar {
| Some(foo) => foo
| None => ""
}
let a = switch a {
let a = switch __a {
| Some(a) => a
| None => bar
}
Expand All @@ -81,8 +81,8 @@ module C3 = {
module C4 = {
type props<'a, 'x> = {a: 'a, x?: 'x}

let make = ({a: b, ?x, _}: props<_, _>) => {
let x = switch x {
let make = ({a: b, x: ?__x, _}: props<_, _>) => {
let x = switch __x {
| Some(x) => x
| None => true
}
Expand All @@ -99,8 +99,8 @@ module C4 = {
module C5 = {
type props<'a, 'z> = {a: 'a, z?: 'z}

let make = ({a: (x, y), ?z, _}: props<_, _>) => {
let z = switch z {
let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
let z = switch __z {
| Some(z) => z
| None => 3
}
Expand Down
49 changes: 44 additions & 5 deletions jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
module C0 = {
type props<'a, 'b> = {a?: 'a, b?: 'b}
let make = ({?a, ?b, _}: props<_, _>) => {
let a = switch a {
let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
let a = switch __a {
| Some(a) => a
| None => 2
}
let b = switch b {
let b = switch __b {
| Some(b) => b
| None => a * 2
}
Expand All @@ -21,8 +21,8 @@ module C0 = {
module C1 = {
type props<'a, 'b> = {a?: 'a, b: 'b}

let make = ({?a, b, _}: props<_, _>) => {
let a = switch a {
let make = ({a: ?__a, b, _}: props<_, _>) => {
let a = switch __a {
| Some(a) => a
| None => 2
}
Expand All @@ -35,3 +35,42 @@ module C1 = {
\"DefaultValueProp$C1"
}
}

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

let make = ({a: ?__a, _}: props<_>) => {
let a = switch __a {
| Some(a) => a
| None => a
}

React.string(a)
}
let make = {
let \"DefaultValueProp$C2" = (props: props<_>) => make(props)

\"DefaultValueProp$C2"
}
}

module C3 = {
type props<'disabled> = {disabled?: 'disabled}

let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
let everythingDisabled = switch __everythingDisabled {
| Some(disabled) => disabled
| None => false
}

{
React.string(everythingDisabled ? "true" : "false")
}
}
let make = {
let \"DefaultValueProp$C3" = (props: props<_>) => make(props)

\"DefaultValueProp$C3"
}
}
8 changes: 4 additions & 4 deletions jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
@@jsxConfig({version: 4})
type props<'a> = {a?: 'a}

let make = ({?a, _}: props<(. unit) => unit>) => {
let a = switch a {
let make = ({a: ?__a, _}: props<(. unit) => unit>) => {
let a = switch __a {
| Some(a) => a
| None => (. ()) => ()
}
Expand All @@ -28,8 +28,8 @@ func(~callback=(. str, a, b) => {
module Foo = {
type props<'callback> = {callback?: 'callback}

let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => {
let callback = switch callback {
let make = ({callback: ?__callback, _}: props<(. string, bool, bool) => unit>) => {
let callback = switch __callback {
| Some(callback) => callback
| None => (. _, _, _) => ()
}
Expand Down
32 changes: 16 additions & 16 deletions jscomp/test/alias_default_value_test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.