Skip to content

fix misparsing in/after JSX (#6677) #6686

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 1 commit into from
Mar 20, 2024
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.1.0-rc.5 (Unreleased)

#### :bug: Bug Fix

- Fix misparsing in/after JSX. https://github.com/rescript-lang/rescript-compiler/pull/6686

# 11.1.0-rc.4

#### :bug: Bug Fix
Expand Down
34 changes: 20 additions & 14 deletions jscomp/syntax/src/res_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2612,6 +2612,7 @@ and parseJsxOpeningOrSelfClosingElement ~startPos p =
let childrenStartPos = p.Parser.startPos in
Parser.next p;
let childrenEndPos = p.Parser.startPos in
Scanner.popMode p.scanner Jsx;
Parser.expect GreaterThan p;
Comment on lines +2615 to 2616
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we expect the end of the JSX so we can pop JSX mode

let loc = mkLoc childrenStartPos childrenEndPos in
makeListExpression loc [] None (* no children *)
Expand All @@ -2621,8 +2622,6 @@ and parseJsxOpeningOrSelfClosingElement ~startPos p =
Parser.next p;
let spread, children = parseJsxChildren p in
let childrenEndPos = p.Parser.startPos in
Scanner.popMode p.scanner Jsx;
Scanner.setJsxMode p.scanner;
let () =
match p.token with
| LessThanSlash -> Parser.next p
Expand All @@ -2634,12 +2633,14 @@ and parseJsxOpeningOrSelfClosingElement ~startPos p =
in
match p.Parser.token with
| (Lident _ | Uident _) when verifyJsxOpeningClosingName p name -> (
Scanner.popMode p.scanner Jsx;
Parser.expect GreaterThan p;
Comment on lines +2636 to 2637
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

let loc = mkLoc childrenStartPos childrenEndPos in
match (spread, children) with
| true, child :: _ -> child
| _ -> makeListExpression loc children None)
| token -> (
Scanner.popMode p.scanner Jsx;
let () =
if Grammar.isStructureItemStart token then
let closing = "</" ^ string_of_pexp_ident name ^ ">" in
Expand All @@ -2660,6 +2661,7 @@ and parseJsxOpeningOrSelfClosingElement ~startPos p =
| true, child :: _ -> child
| _ -> makeListExpression loc children None))
| token ->
Scanner.popMode p.scanner Jsx;
Parser.err p (Diagnostics.unexpected token p.breadcrumbs);
makeListExpression Location.none [] None
in
Expand Down Expand Up @@ -2687,7 +2689,6 @@ and parseJsxOpeningOrSelfClosingElement ~startPos p =
* jsx-children ::= primary-expr* * => 0 or more
*)
and parseJsx p =
Scanner.popMode p.scanner Jsx;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was not needed

Scanner.setJsxMode p.Parser.scanner;
Parser.leaveBreadcrumb p Grammar.Jsx;
let startPos = p.Parser.startPos in
Expand All @@ -2700,7 +2701,6 @@ and parseJsx p =
parseJsxFragment p
| _ -> parseJsxName p
in
Scanner.popMode p.scanner Jsx;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed either

Parser.eatBreadcrumb p;
{jsxExpr with pexp_attributes = [jsxAttr]}

Expand All @@ -2714,9 +2714,10 @@ and parseJsxFragment p =
Parser.expect GreaterThan p;
let _spread, children = parseJsxChildren p in
let childrenEndPos = p.Parser.startPos in
if p.token = LessThan then p.token <- Scanner.reconsiderLessThan p.scanner;
Parser.expect LessThanSlash p;
Parser.expect GreaterThan p;
Scanner.popMode p.scanner Jsx;
Parser.expect GreaterThan p;
Comment on lines +2717 to +2720
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we first check if the next token is not </ instead of just < to test the end of the fragment with a following > (forming a </>), if it's the case then we can pop JSX mode.

let loc = mkLoc childrenStartPos childrenEndPos in
makeListExpression loc children None

Expand Down Expand Up @@ -2768,6 +2769,7 @@ and parseJsxProp p =
Some (label, attrExpr))
(* {...props} *)
| Lbrace -> (
Scanner.popMode p.scanner Jsx;
Parser.next p;
match p.Parser.token with
| DotDotDot -> (
Expand All @@ -2786,6 +2788,7 @@ and parseJsxProp p =
match p.Parser.token with
| Rbrace ->
Parser.next p;
Scanner.setJsxMode p.scanner;
Some (label, attrExpr)
| _ -> None)
| _ -> None)
Expand All @@ -2795,6 +2798,7 @@ and parseJsxProps p =
parseRegion ~grammar:Grammar.JsxAttribute ~f:parseJsxProp p

and parseJsxChildren p =
Scanner.popMode p.scanner Jsx;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSX children should not be parsed in JSX mode because they could be any kind of expression between curly braces.

let rec loop p children =
match p.Parser.token with
| Token.Eof | LessThanSlash -> children
Expand All @@ -2815,21 +2819,23 @@ and parseJsxChildren p =
let () = p.token <- token in
children
| token when Grammar.isJsxChildStart token ->
let () = Scanner.popMode p.scanner Jsx in
let child =
parsePrimaryExpr ~operand:(parseAtomicExpr p) ~noCall:true p
in
loop p (child :: children)
| _ -> children
in
match p.Parser.token with
| DotDotDot ->
Parser.next p;
(true, [parsePrimaryExpr ~operand:(parseAtomicExpr p) ~noCall:true p])
| _ ->
let children = List.rev (loop p []) in
Scanner.popMode p.scanner Jsx;
(false, children)
let spread, children =
match p.Parser.token with
| DotDotDot ->
Parser.next p;
(true, [parsePrimaryExpr ~operand:(parseAtomicExpr p) ~noCall:true p])
| _ ->
let children = List.rev (loop p []) in
(false, children)
in
Scanner.setJsxMode p.scanner;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we're done parsing the children, we reset JSX mode because we have to parse the closing tag.

(spread, children)

and parseBracedOrRecordExpr p =
let startPos = p.Parser.startPos in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,13 @@ let _ =
;;((div ~children:((span ~children:[] ())[@JSX ]) ())[@JSX ])
;;((div ~children:[|a|] ())[@JSX ])
;;((div ~children:(1, 2) ())[@JSX ])
;;((div ~children:((array |. f)[@res.braces ]) ())[@JSX ])
;;(([element])[@JSX ])
;;(([(((fun a -> 1))[@res.braces ])])[@JSX ])
;;(([((span ~children:[] ())[@JSX ])])[@JSX ])
;;(([[|a|]])[@JSX ])
;;(([(1, 2)])[@JSX ])
;;(([((array |. f)[@res.braces ])])[@JSX ])
let _ =
((A.createElement ~x:(({js|y|js})[@res.namedArgLoc ]) ~_spreadProps:((str)
[@res.namedArgLoc ]) ~children:[] ())
Expand Down
2 changes: 2 additions & 0 deletions jscomp/syntax/tests/parsing/grammar/expressions/jsx.res
Original file line number Diff line number Diff line change
Expand Up @@ -507,11 +507,13 @@ let _ = <View style=styles["backgroundImageWrapper"]>
<div> ...<span /> </div>
<div> ...[a] </div>
<div> ...(1, 2) </div>
<div> ...{array->f} </div>

<> ...element </>
<> ...{(a) => 1} </>
<> ...<span /> </>
<> ...[a] </>
<> ...(1, 2) </>
<> ...{array->f} </>

let _ = <A x="y" {...str} />
31 changes: 31 additions & 0 deletions jscomp/syntax/tests/printer/expr/expected/jsx.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,34 @@ let x = props =>
{...props}
className="inline-block px-6 py-2.5 bg-blue-600 text-white font-medium text-xs leading-tight"
/>

let x = <C> ...{() => msg->React.string} </C>

let x = <C> ...{array->Array.map(React.string)} </C>

let x = <> {array->Array.map(React.string)} </>

let x = {
let _ = <div />
msg->React.string
}

let x = {
let _ = <div> {children} </div>
msg->React.string
}

let x = {
let _ = <> {children} </>
msg->React.string
}

let x = {
let _ = <C />
msg->React.string
}

let x = {
let _ = <C> {children} </C>
msg->React.string
}
31 changes: 31 additions & 0 deletions jscomp/syntax/tests/printer/expr/jsx.res
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,34 @@ let x = props =>
{...props}
className="inline-block px-6 py-2.5 bg-blue-600 text-white font-medium text-xs leading-tight"
/>

let x = <C> ...{() => msg->React.string} </C>

let x = <C> ...{array->Array.map(React.string)} </C>

let x = <> ...{array->Array.map(React.string)} </>

let x = {
let _ = <div />
msg->React.string
}

let x = {
let _ = <div> {children} </div>
msg->React.string
}

let x = {
let _ = <> {children} </>
msg->React.string
}

let x = {
let _ = <C />
msg->React.string
}

let x = {
let _ = <C> {children} </C>
msg->React.string
}