From 5ff507e9238ba20afdb52314936eee98d3223372 Mon Sep 17 00:00:00 2001 From: Jon Date: Tue, 5 Dec 2023 10:35:07 -0800 Subject: [PATCH] fix(js_formatter): fix indention when huggable expressions break in arrow chains (#1036) --- .../expressions/arrow_function_expression.rs | 41 +++--- crates/biome_js_formatter/tests/quick_test.rs | 20 +-- .../tests/specs/js/module/arrow/currying.js | 34 +++++ .../specs/js/module/arrow/currying.js.snap | 135 ++++++++++++++++++ 4 files changed, 191 insertions(+), 39 deletions(-) diff --git a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs index a77a7ee5d967..2d535492e27f 100644 --- a/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs +++ b/crates/biome_js_formatter/src/js/expressions/arrow_function_expression.rs @@ -476,6 +476,7 @@ impl Format for ArrowChain { let head_parent = head.syntax().parent(); let tail_body = tail.body()?; let is_assignment_rhs = self.options.assignment_layout.is_some(); + let is_grouped_call_arg_layout = self.options.call_arg_layout.is_some(); let ancestor_call_expr_or_logical_expr = head.syntax().ancestors().any(|ancestor| { matches!( ancestor.kind(), @@ -537,8 +538,6 @@ impl Format for ArrowChain { write!(f, [soft_line_break()])?; } - let is_grouped_call_arg_layout = self.options.call_arg_layout.is_some(); - let join_signatures = format_with(|f: &mut JsFormatter| { let mut is_first_in_chain = true; for arrow in self.arrows() { @@ -637,25 +636,18 @@ impl Format for ArrowChain { } } else { let should_add_parens = should_add_parens(&tail_body); - write!( - f, - [format_with(|f| { - if should_add_parens { - write!( - f, - [ - if_group_fits_on_line(&text("(")), - format_tail_body, - if_group_fits_on_line(&text(")")) - ] - )?; - } else { - write!(f, [format_tail_body])?; - } - - Ok(()) - })] - )?; + if should_add_parens { + write!( + f, + [ + if_group_fits_on_line(&text("(")), + format_tail_body, + if_group_fits_on_line(&text(")")) + ] + )?; + } else { + write!(f, [format_tail_body])?; + } } // Format the trailing comments of all arrow function EXCEPT the first one because @@ -704,10 +696,15 @@ impl Format for ArrowChain { .should_expand(break_signatures), space(), tail.fat_arrow_token().format(), - indent_if_group_breaks(&format_tail_body, group_id), ] )?; + if is_grouped_call_arg_layout { + write!(f, [group(&format_tail_body)])?; + } else { + write!(f, [indent_if_group_breaks(&format_tail_body, group_id)])?; + } + if is_callee { write!( f, diff --git a/crates/biome_js_formatter/tests/quick_test.rs b/crates/biome_js_formatter/tests/quick_test.rs index 526e76dce761..00544a75f4d9 100644 --- a/crates/biome_js_formatter/tests/quick_test.rs +++ b/crates/biome_js_formatter/tests/quick_test.rs @@ -9,27 +9,13 @@ mod language { include!("language.rs"); } -// #[ignore] +#[ignore] #[test] // use this test check if your snippet prints as you wish, without using a snapshot fn quick_test() { let src = r#" - const foo = ({normal: something, n: {yes: what, layout: {h}}}: LE) => { - bar(); - } - - const obj = { - func(id, { blog: { title } }) { - return id + title; - }, - }; - - class A { - func(id, { blog: { title } }) { - return id + title; - } - } - + ((C) => (props) => ); + (({C}) => (props) => ); "#; let source_type = JsFileSource::tsx(); let tree = parse( diff --git a/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js b/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js index 1e3b36d1f172..4c712845371e 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js +++ b/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js @@ -15,3 +15,37 @@ const mw = store => next => action => { const middleware = options => (req, res, next) => { // ... }; + +// Ensure tail bodies only indent a single level when necessary +somePromise.then(({default: ComponentName}) => (props) => [longerSingleElement]); + +somePromise.then((reallyLongArguments) => (makeTheChainFullyBreak) => (moreThanItWould) => [longerSingleElement]); + +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => [ + dontIndentTwice, +]); +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => { + dontIndentTwice(); +}); + +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => (andNowAllLines) => (keepGoing) => + dontIndentTwice()); + + somePromise.then( + ({ reallyLongArguments }) => + (makeTheChainFullyBreak) => + () => { + dontIndentTwice(); + }, + ); + function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => { + callSomeLongNamedFunction(); + }); + } + + function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => [hello, what, is, this, going, too, doehwharht]); + } \ No newline at end of file diff --git a/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js.snap b/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js.snap index d89b31e06e31..0f69fa31ea43 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/arrow/currying.js.snap @@ -24,6 +24,39 @@ const middleware = options => (req, res, next) => { // ... }; +// Ensure tail bodies only indent a single level when necessary +somePromise.then(({default: ComponentName}) => (props) => [longerSingleElement]); + +somePromise.then((reallyLongArguments) => (makeTheChainFullyBreak) => (moreThanItWould) => [longerSingleElement]); + +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => [ + dontIndentTwice, +]); +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => { + dontIndentTwice(); +}); + +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => (andNowAllLines) => (keepGoing) => + dontIndentTwice()); + + somePromise.then( + ({ reallyLongArguments }) => + (makeTheChainFullyBreak) => + () => { + dontIndentTwice(); + }, + ); + function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => { + callSomeLongNamedFunction(); + }); + } + + function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => [hello, what, is, this, going, too, doehwharht]); + } ``` @@ -66,6 +99,59 @@ const mw = (store) => (next) => (action) => { const middleware = (options) => (req, res, next) => { // ... }; + +// Ensure tail bodies only indent a single level when necessary +somePromise.then(({ default: ComponentName }) => (props) => [ + longerSingleElement, +]); + +somePromise.then( + (reallyLongArguments) => (makeTheChainFullyBreak) => (moreThanItWould) => [ + longerSingleElement, + ], +); + +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => [ + dontIndentTwice, +]); +somePromise.then(({ reallyLongArguments }) => (makeTheChainFullyBreak) => { + dontIndentTwice(); +}); + +somePromise.then( + ({ reallyLongArguments }) => + (makeTheChainFullyBreak) => + (andNowAllLines) => + (keepGoing) => + dontIndentTwice(), +); + +somePromise.then( + ({ reallyLongArguments }) => + (makeTheChainFullyBreak) => + () => { + dontIndentTwice(); + }, +); +function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => { + callSomeLongNamedFunction(); + }); +} + +function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => [ + hello, + what, + is, + this, + going, + too, + doehwharht, + ]); +} ``` ## Output 2 @@ -103,6 +189,55 @@ const mw = store => next => action => { const middleware = options => (req, res, next) => { // ... }; + +// Ensure tail bodies only indent a single level when necessary +somePromise.then(({ default: ComponentName }) => props => [ + longerSingleElement, +]); + +somePromise.then( + reallyLongArguments => makeTheChainFullyBreak => moreThanItWould => [ + longerSingleElement, + ], +); + +somePromise.then(({ reallyLongArguments }) => makeTheChainFullyBreak => [ + dontIndentTwice, +]); +somePromise.then(({ reallyLongArguments }) => makeTheChainFullyBreak => { + dontIndentTwice(); +}); + +somePromise.then( + ({ reallyLongArguments }) => + makeTheChainFullyBreak => + andNowAllLines => + keepGoing => + dontIndentTwice(), +); + +somePromise.then(({ reallyLongArguments }) => makeTheChainFullyBreak => () => { + dontIndentTwice(); +}); +function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => { + callSomeLongNamedFunction(); + }); +} + +function foo() { + // Unmount clean up + React.useLayoutEffect(() => () => [ + hello, + what, + is, + this, + going, + too, + doehwharht, + ]); +} ```