Skip to content

Commit 931fa0b

Browse files
committed
Improve code generated for default arguments in JSX V4
This ```res @react.component let foo = (~x, ~y=3+x, ?z) => ... ``` was generating ```res ({props: props<_, _, _>) => { let x = props.x let y = switch props.y { | None => 3 + x | Some(y) => y } let z = props.z ... ``` now it generates instead ``` ({x, ?y, ?z}: props<_, _, _>) => { let x = x let y = switch y { | None => 3 + x | Some(y) => y } let z = z ... ``` the different in generated code is that bindings such as `let x = props.x` are not present in the `js` output.
1 parent e9b20ae commit 931fa0b

9 files changed

+82
-36
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ These are only breaking changes for unformatted code.
7575
- Fix issue with integer overflow check https://github.com/rescript-lang/rescript-compiler/pull/6028
7676
- Fix issue with JSX V4 and newtype https://github.com/rescript-lang/rescript-compiler/pull/6029
7777
- Fix issue with JSX V4 when components are nested https://github.com/rescript-lang/rescript-compiler/pull/6031
78+
- Improve code generated for default arguments in JSX V4 https://github.com/rescript-lang/rescript-compiler/pull/6041
7879

7980
#### :nail_care: Polish
8081

docs/JSXV4.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,19 +264,21 @@ let make = React.forwardRef({
264264
### Transformation for Component Definition
265265

266266
```rescript
267-
@react.component (~x, ~y=3+x, ?z) => body
267+
@react.component (~x, ~y=3+x, ~z=?) => body
268268
```
269269

270270
is transformed to
271271

272272
```rescript
273273
type props<'x, 'y, 'z> = {x: 'x, y?: 'y, z?: 'z}
274274
275-
({x, y, z}: props<_>) => {
276-
let y = switch props.y {
275+
({x, ?y, ?z}: props<_, _, _>) => {
276+
let x = x
277+
let y = switch y {
277278
| None => 3 + x
278279
| Some(y) => y
279280
}
281+
let z = z
280282
body
281283
}
282284
```

jscomp/test/alias_default_value_test.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33

44
function Alias_default_value_test$C0(props) {
5+
var b = props.b;
56
var a = props.a;
67
var a$1 = a !== undefined ? a : 2;
7-
var b = props.b;
88
var b$1 = b !== undefined ? b : (a$1 << 1);
99
return a$1 + b$1 | 0;
1010
}
@@ -14,9 +14,9 @@ var C0 = {
1414
};
1515

1616
function Alias_default_value_test$C1(props) {
17-
var foo = props.foo;
18-
if (foo !== undefined) {
19-
return foo;
17+
var bar = props.foo;
18+
if (bar !== undefined) {
19+
return bar;
2020
} else {
2121
return "";
2222
}
@@ -26,6 +26,19 @@ var C1 = {
2626
make: Alias_default_value_test$C1
2727
};
2828

29+
function Alias_default_value_test$C2(props) {
30+
var a = props.a;
31+
var bar = props.foo;
32+
var bar$1 = bar !== undefined ? bar : "";
33+
var a$1 = a !== undefined ? a : bar$1;
34+
return bar$1 + a$1 + props.b;
35+
}
36+
37+
var C2 = {
38+
make: Alias_default_value_test$C2
39+
};
40+
2941
exports.C0 = C0;
3042
exports.C1 = C1;
43+
exports.C2 = C2;
3144
/* No side effect */

jscomp/test/alias_default_value_test.res

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,10 @@ module C1 = {
1515
React.string(bar)
1616
}
1717
}
18+
19+
module C2 = {
20+
@react.component
21+
let make = (~foo as bar="", ~a=bar, ~b) => {
22+
React.string(bar ++ a ++ b)
23+
}
24+
}

res_syntax/src/reactjs_jsx_v4.ml

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -850,9 +850,7 @@ let vbMatch (name, default, _, alias, loc, _) =
850850
Vb.mk
851851
(Pat.var (Location.mkloc alias loc))
852852
(Exp.match_
853-
(Exp.field
854-
(Exp.ident {txt = Lident "props"; loc = Location.none})
855-
(Location.mknoloc @@ Lident label))
853+
(Exp.ident {txt = Lident alias; loc = Location.none})
856854
[
857855
Exp.case
858856
(Pat.construct
@@ -866,9 +864,7 @@ let vbMatch (name, default, _, alias, loc, _) =
866864
| None ->
867865
Vb.mk
868866
(Pat.var (Location.mkloc alias loc))
869-
(Exp.field
870-
(Exp.ident {txt = Lident "props"; loc = Location.none})
871-
(Location.mknoloc @@ Lident label))
867+
(Exp.ident {txt = Lident label; loc = Location.none})
872868

873869
let vbMatchExpr namedArgList expr =
874870
let rec aux namedArgList =
@@ -1043,11 +1039,6 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
10431039
| [] -> Pat.any ()
10441040
| _ -> Pat.record (List.rev patternsWithLabel) Open
10451041
in
1046-
let recordPattern =
1047-
if hasDefaultValue namedArgList then
1048-
Pat.var {txt = "props"; loc = emptyLoc}
1049-
else recordPattern
1050-
in
10511042
let expression =
10521043
Exp.fun_ Nolabel None
10531044
(Pat.constraint_ recordPattern

res_syntax/tests/ppx/react/aliasProps.res

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,10 @@ module C2 = {
1414
@react.component
1515
let make = (~foo as bar="") => React.string(bar)
1616
}
17+
18+
module C3 = {
19+
@react.component
20+
let make = (~foo as bar="", ~a=bar, ~b) => {
21+
React.string(bar ++ a ++ b)
22+
}
23+
}

res_syntax/tests/ppx/react/expected/aliasProps.res.txt

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

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

25-
let make = (props: props<_, _>) => {
26-
let p = props.priority
27-
let text = switch props.text {
25+
let make = ({priority: p, ?text, _}: props<_, _>) => {
26+
let p = priority
27+
let text = switch text {
2828
| Some(text) => text
2929
| None => "Test"
3030
}
@@ -41,8 +41,8 @@ module C1 = {
4141
module C2 = {
4242
type props<'foo> = {foo?: 'foo}
4343

44-
let make = (props: props<_>) => {
45-
let bar = switch props.foo {
44+
let make = ({foo: ?bar, _}: props<_>) => {
45+
let bar = switch bar {
4646
| Some(foo) => foo
4747
| None => ""
4848
}
@@ -55,3 +55,28 @@ module C2 = {
5555
\"AliasProps$C2"
5656
}
5757
}
58+
59+
module C3 = {
60+
type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
61+
62+
let make = ({foo: ?bar, ?a, b, _}: props<_, _, _>) => {
63+
let bar = switch bar {
64+
| Some(foo) => foo
65+
| None => ""
66+
}
67+
let a = switch a {
68+
| Some(a) => a
69+
| None => bar
70+
}
71+
let b = b
72+
73+
{
74+
React.string(bar ++ a ++ b)
75+
}
76+
}
77+
let make = {
78+
let \"AliasProps$C3" = (props: props<_>) => make(props)
79+
80+
\"AliasProps$C3"
81+
}
82+
}

res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt

Lines changed: 6 additions & 6 deletions
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 = (props: props<_, _>) => {
4-
let a = switch props.a {
3+
let make = ({?a, ?b, _}: props<_, _>) => {
4+
let a = switch a {
55
| Some(a) => a
66
| None => 2
77
}
8-
let b = switch props.b {
8+
let b = switch b {
99
| Some(b) => b
1010
| None => a * 2
1111
}
@@ -21,12 +21,12 @@ module C0 = {
2121
module C1 = {
2222
type props<'a, 'b> = {a?: 'a, b: 'b}
2323

24-
let make = (props: props<_, _>) => {
25-
let a = switch props.a {
24+
let make = ({?a, b, _}: props<_, _>) => {
25+
let a = switch a {
2626
| Some(a) => a
2727
| None => 2
2828
}
29-
let b = props.b
29+
let b = b
3030

3131
React.int(a + b)
3232
}

res_syntax/tests/ppx/react/expected/uncurriedProps.res.txt

Lines changed: 4 additions & 4 deletions
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 = (props: props<(. unit) => unit>) => {
5-
let a = switch props.a {
4+
let make = ({?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 = (props: props<(. string, bool, bool) => unit>) => {
32-
let callback = switch props.callback {
31+
let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => {
32+
let callback = switch callback {
3333
| Some(callback) => callback
3434
| None => (. _, _, _) => ()
3535
}

0 commit comments

Comments
 (0)