Skip to content

Commit c43223f

Browse files
authored
Fix JSX V4 build error when component props have the default value with same name (#6377)
1 parent 8de6c09 commit c43223f

7 files changed

+122
-42
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
1313
# 11.0.0-rc.3 (Unreleased)
1414

15+
#### :bug: Bug Fix
16+
17+
- Fix issue with JSX V4 when component props have the default value with same name. https://github.com/rescript-lang/rescript-compiler/pull/6377
18+
1519
# 11.0.0-rc.2
1620

1721
#### :rocket: New Feature

jscomp/syntax/src/reactjs_jsx_v4.ml

+28-4
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,13 @@ let rec recursivelyTransformNamedArgsForMake expr args newtypes coreType =
634634
in
635635
let alias =
636636
match pattern with
637-
| {ppat_desc = Ppat_alias (_, {txt}) | Ppat_var {txt}} -> txt
637+
| {
638+
ppat_desc =
639+
( Ppat_alias (_, {txt})
640+
| Ppat_var {txt}
641+
| Ppat_constraint ({ppat_desc = Ppat_var {txt}}, _) );
642+
} ->
643+
txt
638644
| {ppat_desc = Ppat_any} -> "_"
639645
| _ -> getLabel arg
640646
in
@@ -852,7 +858,7 @@ let vbMatch ~expr (name, default, _, alias, loc, _) =
852858
Vb.mk
853859
(Pat.var (Location.mkloc alias loc))
854860
(Exp.match_
855-
(Exp.ident {txt = Lident alias; loc = Location.none})
861+
(Exp.ident {txt = Lident ("__" ^ alias); loc = Location.none})
856862
[
857863
Exp.case
858864
(Pat.construct
@@ -978,6 +984,14 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
978984
stripConstraintUnpack ~label pattern
979985
| _ -> pattern
980986
in
987+
let safePatternLabel pattern =
988+
match pattern with
989+
| {ppat_desc = Ppat_var {txt; loc}} ->
990+
{pattern with ppat_desc = Ppat_var {txt = "__" ^ txt; loc}}
991+
| {ppat_desc = Ppat_alias (p, {txt; loc})} ->
992+
{pattern with ppat_desc = Ppat_alias (p, {txt = "__" ^ txt; loc})}
993+
| _ -> pattern
994+
in
981995
let rec returnedExpression patternsWithLabel patternsWithNolabel
982996
({pexp_desc} as expr) =
983997
match pexp_desc with
@@ -991,16 +1005,26 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
9911005
{ppat_desc = Ppat_construct ({txt = Lident "()"}, _)},
9921006
expr ) ->
9931007
(patternsWithLabel, patternsWithNolabel, expr)
994-
| Pexp_fun (arg_label, _default, ({ppat_loc; ppat_desc} as pattern), expr)
1008+
| Pexp_fun (arg_label, default, ({ppat_loc; ppat_desc} as pattern), expr)
9951009
-> (
9961010
let patternWithoutConstraint =
9971011
stripConstraintUnpack ~label:(getLabel arg_label) pattern
9981012
in
1013+
(*
1014+
If prop has the default value as Ident, it will get a build error
1015+
when the referenced Ident value and the prop have the same name.
1016+
So we add a "__" to label to resolve the build error.
1017+
*)
1018+
let patternWithSafeLabel =
1019+
match default with
1020+
| Some _ -> safePatternLabel patternWithoutConstraint
1021+
| _ -> patternWithoutConstraint
1022+
in
9991023
if isLabelled arg_label || isOptional arg_label then
10001024
returnedExpression
10011025
(( {loc = ppat_loc; txt = Lident (getLabel arg_label)},
10021026
{
1003-
patternWithoutConstraint with
1027+
patternWithSafeLabel with
10041028
ppat_attributes =
10051029
(if isOptional arg_label then optionalAttrs else [])
10061030
@ pattern.ppat_attributes;

jscomp/syntax/tests/ppx/react/defaultValueProp.res

+13
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,16 @@ module C1 = {
77
@react.component
88
let make = (~a=2, ~b) => React.int(a + b)
99
}
10+
11+
module C2 = {
12+
let a = "foo"
13+
@react.component
14+
let make = (~a=a) => React.string(a)
15+
}
16+
17+
module C3 = {
18+
@react.component
19+
let make = (~disabled as everythingDisabled: bool=false) => {
20+
React.string(everythingDisabled ? "true" : "false")
21+
}
22+
}

jscomp/syntax/tests/ppx/react/expected/aliasProps.res.txt

+13-13
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
module C0 = {
44
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
55

6-
let make = ({priority: _, ?text, _}: props<_, _>) => {
7-
let text = switch text {
6+
let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
7+
let text = switch __text {
88
| Some(text) => text
99
| None => "Test"
1010
}
@@ -21,8 +21,8 @@ module C0 = {
2121
module C1 = {
2222
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
2323

24-
let make = ({priority: p, ?text, _}: props<_, _>) => {
25-
let text = switch text {
24+
let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
25+
let text = switch __text {
2626
| Some(text) => text
2727
| None => "Test"
2828
}
@@ -39,8 +39,8 @@ module C1 = {
3939
module C2 = {
4040
type props<'foo> = {foo?: 'foo}
4141

42-
let make = ({foo: ?bar, _}: props<_>) => {
43-
let bar = switch bar {
42+
let make = ({foo: ?__bar, _}: props<_>) => {
43+
let bar = switch __bar {
4444
| Some(foo) => foo
4545
| None => ""
4646
}
@@ -57,12 +57,12 @@ module C2 = {
5757
module C3 = {
5858
type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
5959

60-
let make = ({foo: ?bar, ?a, b, _}: props<_, _, _>) => {
61-
let bar = switch bar {
60+
let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
61+
let bar = switch __bar {
6262
| Some(foo) => foo
6363
| None => ""
6464
}
65-
let a = switch a {
65+
let a = switch __a {
6666
| Some(a) => a
6767
| None => bar
6868
}
@@ -81,8 +81,8 @@ module C3 = {
8181
module C4 = {
8282
type props<'a, 'x> = {a: 'a, x?: 'x}
8383

84-
let make = ({a: b, ?x, _}: props<_, _>) => {
85-
let x = switch x {
84+
let make = ({a: b, x: ?__x, _}: props<_, _>) => {
85+
let x = switch __x {
8686
| Some(x) => x
8787
| None => true
8888
}
@@ -99,8 +99,8 @@ module C4 = {
9999
module C5 = {
100100
type props<'a, 'z> = {a: 'a, z?: 'z}
101101

102-
let make = ({a: (x, y), ?z, _}: props<_, _>) => {
103-
let z = switch z {
102+
let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
103+
let z = switch __z {
104104
| Some(z) => z
105105
| None => 3
106106
}

jscomp/syntax/tests/ppx/react/expected/defaultValueProp.res.txt

+44-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
module C0 = {
22
type props<'a, 'b> = {a?: 'a, b?: 'b}
3-
let make = ({?a, ?b, _}: props<_, _>) => {
4-
let a = switch a {
3+
let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
4+
let a = switch __a {
55
| Some(a) => a
66
| None => 2
77
}
8-
let b = switch b {
8+
let b = switch __b {
99
| Some(b) => b
1010
| None => a * 2
1111
}
@@ -21,8 +21,8 @@ module C0 = {
2121
module C1 = {
2222
type props<'a, 'b> = {a?: 'a, b: 'b}
2323

24-
let make = ({?a, b, _}: props<_, _>) => {
25-
let a = switch a {
24+
let make = ({a: ?__a, b, _}: props<_, _>) => {
25+
let a = switch __a {
2626
| Some(a) => a
2727
| None => 2
2828
}
@@ -35,3 +35,42 @@ module C1 = {
3535
\"DefaultValueProp$C1"
3636
}
3737
}
38+
39+
module C2 = {
40+
let a = "foo"
41+
type props<'a> = {a?: 'a}
42+
43+
let make = ({a: ?__a, _}: props<_>) => {
44+
let a = switch __a {
45+
| Some(a) => a
46+
| None => a
47+
}
48+
49+
React.string(a)
50+
}
51+
let make = {
52+
let \"DefaultValueProp$C2" = (props: props<_>) => make(props)
53+
54+
\"DefaultValueProp$C2"
55+
}
56+
}
57+
58+
module C3 = {
59+
type props<'disabled> = {disabled?: 'disabled}
60+
61+
let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
62+
let everythingDisabled = switch __everythingDisabled {
63+
| Some(disabled) => disabled
64+
| None => false
65+
}
66+
67+
{
68+
React.string(everythingDisabled ? "true" : "false")
69+
}
70+
}
71+
let make = {
72+
let \"DefaultValueProp$C3" = (props: props<_>) => make(props)
73+
74+
\"DefaultValueProp$C3"
75+
}
76+
}

jscomp/syntax/tests/ppx/react/expected/uncurriedProps.res.txt

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
@@jsxConfig({version: 4})
22
type props<'a> = {a?: 'a}
33

4-
let make = ({?a, _}: props<(. unit) => unit>) => {
5-
let a = switch a {
4+
let make = ({a: ?__a, _}: props<(. unit) => unit>) => {
5+
let a = switch __a {
66
| Some(a) => a
77
| None => (. ()) => ()
88
}
@@ -28,8 +28,8 @@ func(~callback=(. str, a, b) => {
2828
module Foo = {
2929
type props<'callback> = {callback?: 'callback}
3030

31-
let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => {
32-
let callback = switch callback {
31+
let make = ({callback: ?__callback, _}: props<(. string, bool, bool) => unit>) => {
32+
let callback = switch __callback {
3333
| Some(callback) => callback
3434
| None => (. _, _, _) => ()
3535
}

jscomp/test/alias_default_value_test.js

+16-16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)