From a25a05add9d730a394a1feb99f090ba5db4e61b2 Mon Sep 17 00:00:00 2001 From: Aaron Lieberman Date: Sun, 20 Jul 2025 22:24:34 -0700 Subject: [PATCH] fix: Improve contextual spacing for JSX expression containers when jsxExpressionContainer.spaceSurroundingExpression: true When jsxExpressionContainer.spaceSurroundingExpression is true, make sure the spaces around braces are right based on the context of where the brace lies. The change is to only add the space if the open brace is not at the end of a line. We do that on the opening brace by adding a Signal::SpaceOrNewLine which allows the system to add the space if it needs to for purposes of word wrap, otherwise ommitting it. For the close brace, we add the space if we're not the first token on the line. The impetus for this was the case where an opening brace within a JSX element was the last character on a line, or a closing brace was the first. For example: ``` } />; ``` The above case was getting formatted with extra spaces around the braces for the "inner" attribute. This fixes this case and similar ones, plus adds additional varied test cases to cover variations of this idea. --- src/generation/generate.rs | 12 +- ...tainer_SpaceSurroundingExpression_True.txt | 190 ++++++++++++++++++ 2 files changed, 196 insertions(+), 6 deletions(-) diff --git a/src/generation/generate.rs b/src/generation/generate.rs index f978a04b..52c19481 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -3949,16 +3949,16 @@ fn gen_as_jsx_expr_container(expr: Node, inner_items: PrintItems, context: &mut if surround_with_new_lines { items.push_signal(Signal::NewLine); items.push_signal(Signal::StartIndent); - } else if surround_with_space { - items.push_space(); - } - items.extend(inner_items); - if surround_with_new_lines { + items.extend(inner_items); items.extend(gen_trailing_comments_as_statements(&expr.range(), context)); items.push_signal(Signal::NewLine); items.push_signal(Signal::FinishIndent); } else if surround_with_space { - items.push_space(); + items.push_signal(Signal::SpaceOrNewLine); + items.extend(inner_items); + items.push_condition(if_false("addSpaceIfNotStartOfLine", condition_resolvers::is_start_of_line(), " ".into())); + } else { + items.extend(inner_items); } items.push_sc(sc!("}")); diff --git a/tests/specs/jsx/JsxExpressionContainer/JsxExpressionContainer_SpaceSurroundingExpression_True.txt b/tests/specs/jsx/JsxExpressionContainer/JsxExpressionContainer_SpaceSurroundingExpression_True.txt index 15a7b4a9..32ba7728 100644 --- a/tests/specs/jsx/JsxExpressionContainer/JsxExpressionContainer_SpaceSurroundingExpression_True.txt +++ b/tests/specs/jsx/JsxExpressionContainer/JsxExpressionContainer_SpaceSurroundingExpression_True.txt @@ -5,3 +5,193 @@ const t = ; [expect] const t = ; + +== should not add extra space before closing brace in nested JSX == + + } +/>; + +[expect] + + } +/>; + +== should not add extra space in similar nested JSX pattern == + + } + otherProp="value" +/>; + +[expect] + + } + otherProp="value" +/>; + +== should handle deeply nested JSX without extra spaces == + + } + /> + } +/>; + +[expect] + + } + /> + } +/>; + +== expression braces on the same line as a component == +{contents}; + +[expect] +{ contents }; + +== expression braces on the same line == +return ( +{contents}); + +[expect] +return ( + + { contents } + +); + +== expression braces across multiple lines == +return {
}; + +[expect] +return ( + + { +
+
+
+ } + +); + +== open braces followed by open braces == +return ( + +); + +[expect] +return ( + +); + +== open braces with compound expressions == +<> + {groups.length === 0 && ( +
+

text

+
+ )} +; + +[expect] +<> + { groups.length === 0 && ( +
+

text

+
+ ) } +; + +== open braces with arrow function == +<> + {data.map((data, index) => ( +
+ { data.label }: { data.text } +
+ ))} +; + +[expect] +<> + { data.map((data, index) => ( +
+ { data.label }: { data.text } +
+ )) } +; + +== more varied formatting == + + } + /> + +[expect] + } +/>; + +== more varied formatting across lines == + + } + /> + +[expect] + + } +/>;