-
Notifications
You must be signed in to change notification settings - Fork 664
feat(rome_js_formatter): Binary like object literal member expression #2688
feat(rome_js_formatter): Binary like object literal member expression #2688
Conversation
stateIsOK && | ||
{ | ||
loadState: LOADED, | ||
opened: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't added this case to spec because it still has the wrong print.
Playground
/// Determines if a binary like expression should be inline or not. | ||
pub fn should_inline(&self) -> bool { | ||
match self { | ||
JsAnyBinaryLikeExpression::JsLogicalExpression(logical_expression) => { | ||
match logical_expression.right() { | ||
Ok(JsAnyExpression::JsObjectExpression(object_expression)) => { | ||
object_expression.members().iter().count() > 0 | ||
} | ||
Ok(JsAnyExpression::JsArrayExpression(array_expression)) => { | ||
array_expression.elements().iter().count() > 0 | ||
} | ||
_ => false, | ||
} | ||
} | ||
_ => false, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fn should_not_indent_if_parent_indents(current_node: &JsAnyBinaryLikeLeftExpression) -> bool { | ||
let parent_kind = current_node.syntax().parent().map(|parent| parent.kind()); | ||
|
||
match parent_kind { | ||
Some(JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER) => current_node | ||
.as_expression() | ||
.and_then(|expression| is_break_after_colon(expression).ok()) | ||
.unwrap_or(false), | ||
Some(JsSyntaxKind::JS_RETURN_STATEMENT | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION) => { | ||
true | ||
} | ||
_ => false, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by prettier code, because it duplicates conditionals from assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe it's the other way around, assignments formatting logic that is borrowed from binary like formatting logic
Thank you @Dominionys for you contribution! Could you restore the PR template and add the "Test Plan" section, please? This helps the reviewers to understand how you tested (or you plan to verify) your new changes and highlight possible known regressions. |
…teral_member_expression # Conflicts: # crates/rome_js_formatter/src/utils/binary_like_expression.rs
Summary
Rome adds an extra indent for a binary like object literal member expressions.
Added
should_inline
forJsAnyBinaryLikeExpression
.Added a new conditional branch which checks that binary like expression is a value of
PROPERTY_OBJECT_MEMBER
inshould_not_indent_if_parent_indents
forFlattenedBinaryExpressionPart
.Input
Prettier
Rome
Test Plan
New tests to cover the case.
Added new tests cases for
crates/rome_js_formatter/tests/specs/js/module/object/property_object_member.js
and