Skip to content

Commit ef0db6b

Browse files
jsmechamclaude
jsmecham
and
claude
authored
fix(formatter): sequence expression in arrow function body collapses onto one line (#21183)
## Summary Fixes #21171 When a sequence expression (comma operator) is used as the body of a parenthesized arrow function, the formatter incorrectly collapsed all items onto a single line even when the content was too wide to fit at the arrow function's column. ### Input ```js const result = items.reduce( (acc, item) => ( isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined, acc ), {} ); ``` ### Before (incorrect) ```js const result = items.reduce( (acc, item) => ( isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined, acc ), {}, ); ``` ### After (matches Prettier) ```js const result = items.reduce( (acc, item) => ( isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined, acc ), {}, ); ``` Short sequences that fit on one line are unaffected: ```js // (acc[item.id] = item.value), acc — fits at the arrow column → stays on one line const result = items.reduce( (acc, item) => ((acc[item.id] = item.value), acc), {}, ); ``` ## Root cause The formatter wrapped sequence expression items in their own `group` and the arrow body handler wrapped that group in `soft_block_indent`. When the arrow body's `(...)` group broke, `soft_block_indent` indented the sequence group to column 4 (inside the parens). At that indented position, the items fit within the print width — so the sequence group never broke and items stayed on one line. Prettier avoids this by placing the `(` and `)` *outside* the sequence group, so the group measures fitness starting at the arrow-function column (before `(`). That higher column makes long sequences exceed the print width and break correctly. ## Fix Two coordinated changes: 1. **`sequence_expression.rs`** — when the parent is an arrow function body, wrap items in `soft_block_indent` *inside* the group. This gives the group the same break/indent semantics as Prettier's `group(ifBreak([indent([softline, parts]), softline], parts))`. 2. **`arrow_function_expression.rs`** — remove the outer `soft_block_indent` from the sequence body handler (both `Single` and `Chain` layouts). The sequence group's own `soft_block_indent` is now the sole source of indentation, and the group's break decision is made at the column of `(` — the correct reference point. Prettier conformance: **no regressions** (745/753 JS, 590/601 TS — unchanged). ## AI Disclosure This PR was co-authored with Claude Code (AI assistant), as noted in the commit. The fix was reviewed, tested against the full Prettier conformance suite, and verified to produce no regressions. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5d5d808 commit ef0db6b

4 files changed

Lines changed: 217 additions & 15 deletions

File tree

‎crates/oxc_formatter/src/print/arrow_function_expression.rs‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,10 @@ impl<'a, 'b> FormatJsArrowFunctionExpression<'a, 'b> {
153153
f,
154154
[group(&format_args!(
155155
formatted_signature,
156-
group(&format_args!(
157-
space(),
158-
token("("),
159-
soft_block_indent(&format_body),
160-
token(")")
161-
))
156+
space(),
157+
token("("),
158+
format_body,
159+
token(")")
162160
))]
163161
);
164162
};
@@ -625,14 +623,7 @@ impl<'a> Format<'a> for ArrowChain<'a, '_> {
625623
{
626624
write!(f, format_sequence);
627625
} else {
628-
write!(
629-
f,
630-
[group(&format_args!(
631-
token("("),
632-
soft_block_indent(&format_tail_body),
633-
token(")")
634-
))]
635-
);
626+
write!(f, [token("("), format_tail_body, token(")")]);
636627
}
637628
} else {
638629
let should_add_parens = tail.expression && should_add_parens(tail_body);

‎crates/oxc_formatter/src/print/sequence_expression.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ use super::FormatWrite;
1010

1111
impl<'a> FormatWrite<'a> for AstNode<'a, SequenceExpression<'a>> {
1212
fn write(&self, f: &mut Formatter<'_, 'a>) {
13+
let is_arrow_body = matches!(
14+
self.parent(),
15+
AstNodes::ExpressionStatement(statement) if statement.is_arrow_function_body()
16+
);
17+
1318
let format_inner = format_with(|f| {
1419
let mut expressions = self.expressions().iter();
1520
let separator = format_with(|f| {
@@ -39,6 +44,13 @@ impl<'a> FormatWrite<'a> for AstNode<'a, SequenceExpression<'a>> {
3944
}
4045
});
4146

42-
write!(f, group(&format_inner));
47+
// For arrow bodies, own the `soft_block_indent` so the break decision is made
48+
// at the opening `(`, not at the already-indented column inside it. The arrow
49+
// body handler skips its own indent to defer to this group.
50+
if is_arrow_body {
51+
write!(f, group(&soft_block_indent(&format_inner)));
52+
} else {
53+
write!(f, group(&format_inner));
54+
}
4355
}
4456
}

‎crates/oxc_formatter/tests/fixtures/js/sequence-expression/issue-21171.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Issue #21171 - sequence expression in arrow function body
2+
// Short sequence: should collapse to one line
3+
const result = items.reduce(
4+
(acc, item) => (
5+
(acc[item.id] = item.value),
6+
acc
7+
),
8+
{}
9+
);
10+
11+
// Long sequence: should break to separate lines
12+
const result2 = items.reduce(
13+
(acc, item) => (
14+
isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined,
15+
acc
16+
),
17+
{}
18+
);
19+
20+
// Three-item short sequence: should collapse
21+
const result3 = (a, b) => (a++, b++, a + b);
22+
23+
// Three-item long sequence: should break
24+
const result4 = (a, b) => (
25+
someVeryLongFunctionName(a),
26+
anotherVeryLongFunctionName(b),
27+
yetAnotherVeryLongFunctionName(a, b)
28+
);
29+
30+
// Arrow chain returning a sequence (long): should break inside the tail
31+
const chained = (a) => (b) => (c) => (
32+
isLong(a) ? (state[a] = { x: a, y: b, z: c }) : undefined,
33+
state
34+
);
35+
36+
// Arrow chain returning a short sequence: should collapse
37+
const chained2 = (a) => (b) => ((state[a] = b), state);
38+
39+
// Sequence as a top-level expression statement (not an arrow body):
40+
// behavior should be unchanged by this fix.
41+
a, b, c;
42+
firstLongIdentifier, secondLongIdentifier, thirdLongIdentifier, fourthLongIdentifier;
43+
44+
// Sequence inside a for-loop update clause: behavior should be unchanged.
45+
for (let i = 0, j = 10; i < j; i++, j--) {
46+
doSomething(i, j);
47+
}

‎crates/oxc_formatter/tests/fixtures/js/sequence-expression/issue-21171.js.snap‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
---
2+
source: crates/oxc_formatter/tests/fixtures/mod.rs
3+
---
4+
==================== Input ====================
5+
// Issue #21171 - sequence expression in arrow function body
6+
// Short sequence: should collapse to one line
7+
const result = items.reduce(
8+
(acc, item) => (
9+
(acc[item.id] = item.value),
10+
acc
11+
),
12+
{}
13+
);
14+
15+
// Long sequence: should break to separate lines
16+
const result2 = items.reduce(
17+
(acc, item) => (
18+
isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined,
19+
acc
20+
),
21+
{}
22+
);
23+
24+
// Three-item short sequence: should collapse
25+
const result3 = (a, b) => (a++, b++, a + b);
26+
27+
// Three-item long sequence: should break
28+
const result4 = (a, b) => (
29+
someVeryLongFunctionName(a),
30+
anotherVeryLongFunctionName(b),
31+
yetAnotherVeryLongFunctionName(a, b)
32+
);
33+
34+
// Arrow chain returning a sequence (long): should break inside the tail
35+
const chained = (a) => (b) => (c) => (
36+
isLong(a) ? (state[a] = { x: a, y: b, z: c }) : undefined,
37+
state
38+
);
39+
40+
// Arrow chain returning a short sequence: should collapse
41+
const chained2 = (a) => (b) => ((state[a] = b), state);
42+
43+
// Sequence as a top-level expression statement (not an arrow body):
44+
// behavior should be unchanged by this fix.
45+
a, b, c;
46+
firstLongIdentifier, secondLongIdentifier, thirdLongIdentifier, fourthLongIdentifier;
47+
48+
// Sequence inside a for-loop update clause: behavior should be unchanged.
49+
for (let i = 0, j = 10; i < j; i++, j--) {
50+
doSomething(i, j);
51+
}
52+
53+
==================== Output ====================
54+
------------------
55+
{ printWidth: 80 }
56+
------------------
57+
// Issue #21171 - sequence expression in arrow function body
58+
// Short sequence: should collapse to one line
59+
const result = items.reduce(
60+
(acc, item) => ((acc[item.id] = item.value), acc),
61+
{},
62+
);
63+
64+
// Long sequence: should break to separate lines
65+
const result2 = items.reduce(
66+
(acc, item) => (
67+
isLong(item)
68+
? (acc[item.id] = { key: item.id, value: item.value })
69+
: undefined,
70+
acc
71+
),
72+
{},
73+
);
74+
75+
// Three-item short sequence: should collapse
76+
const result3 = (a, b) => (a++, b++, a + b);
77+
78+
// Three-item long sequence: should break
79+
const result4 = (a, b) => (
80+
someVeryLongFunctionName(a),
81+
anotherVeryLongFunctionName(b),
82+
yetAnotherVeryLongFunctionName(a, b)
83+
);
84+
85+
// Arrow chain returning a sequence (long): should break inside the tail
86+
const chained = (a) => (b) => (c) => (
87+
isLong(a) ? (state[a] = { x: a, y: b, z: c }) : undefined,
88+
state
89+
);
90+
91+
// Arrow chain returning a short sequence: should collapse
92+
const chained2 = (a) => (b) => ((state[a] = b), state);
93+
94+
// Sequence as a top-level expression statement (not an arrow body):
95+
// behavior should be unchanged by this fix.
96+
(a, b, c);
97+
(firstLongIdentifier,
98+
secondLongIdentifier,
99+
thirdLongIdentifier,
100+
fourthLongIdentifier);
101+
102+
// Sequence inside a for-loop update clause: behavior should be unchanged.
103+
for (let i = 0, j = 10; i < j; i++, j--) {
104+
doSomething(i, j);
105+
}
106+
107+
-------------------
108+
{ printWidth: 100 }
109+
-------------------
110+
// Issue #21171 - sequence expression in arrow function body
111+
// Short sequence: should collapse to one line
112+
const result = items.reduce((acc, item) => ((acc[item.id] = item.value), acc), {});
113+
114+
// Long sequence: should break to separate lines
115+
const result2 = items.reduce(
116+
(acc, item) => (
117+
isLong(item) ? (acc[item.id] = { key: item.id, value: item.value }) : undefined,
118+
acc
119+
),
120+
{},
121+
);
122+
123+
// Three-item short sequence: should collapse
124+
const result3 = (a, b) => (a++, b++, a + b);
125+
126+
// Three-item long sequence: should break
127+
const result4 = (a, b) => (
128+
someVeryLongFunctionName(a),
129+
anotherVeryLongFunctionName(b),
130+
yetAnotherVeryLongFunctionName(a, b)
131+
);
132+
133+
// Arrow chain returning a sequence (long): should break inside the tail
134+
const chained = (a) => (b) => (c) => (
135+
isLong(a) ? (state[a] = { x: a, y: b, z: c }) : undefined,
136+
state
137+
);
138+
139+
// Arrow chain returning a short sequence: should collapse
140+
const chained2 = (a) => (b) => ((state[a] = b), state);
141+
142+
// Sequence as a top-level expression statement (not an arrow body):
143+
// behavior should be unchanged by this fix.
144+
(a, b, c);
145+
(firstLongIdentifier, secondLongIdentifier, thirdLongIdentifier, fourthLongIdentifier);
146+
147+
// Sequence inside a for-loop update clause: behavior should be unchanged.
148+
for (let i = 0, j = 10; i < j; i++, j--) {
149+
doSomething(i, j);
150+
}
151+
152+
===================== End =====================

0 commit comments

Comments
 (0)