tools
tools copied to clipboard
feat(rome_js_formatter): Binary expression formatting
Summary
Part of #3046
This PR refactors our JsAnyBinaryLikeExpression formatting to closer match prettier's formatting. The main change is that our old implementation used to group the operator with the left hand side:
a && b && c
became
group("a &&", [group("b &&", c)])
whereas prettier (and this PR) always groups the operator with the right-hand side
"a", group("&& b", group(" && c"))
This PR further enables the NeedsParentheses implementation for binary expressions.
Unsupported case
The new implementation doesn't support the issue-7024 test case
const radioSelectedAttr =
(isAnyValueSelected &&
node.getAttribute(radioAttr.toLowerCase()) === radioValue) ||
((!isAnyValueSelected && values[a].default === true) || a === 0);
The issue is that removing the parentheses from ((!isAnyValueSelected && values[a].default === true) || a === 0) rebalances the tree. Prettier solves this problem by rebalancing logical expressions ahead of formatting. This is something Rome doesn't support today which is why I disabled parentheses removal from logical expression if the child and parent have the same operator but the child is the right side expression.
Test Plan
I manually reviewed the snapshot changes.
Average compatibility: 81.64 -> 82.59 Compatible lines: 78.42 -> 79.70
Deploying with
Cloudflare Pages
| Latest commit: |
db28af0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dc29ad6a.tools-8rn.pages.dev |
| Branch Preview URL: | https://feat-binary-expressions.tools-8rn.pages.dev |
What does "re-balance a tree" mean?
What does "re-balance a tree" mean?
To change the tree's shape without changing the semantic meaning. In this particular case, it means that a tree that has nested logical expressions in the right side to the left side so that the right side.
&&
a &&
b c
// becomes
&&
&& c
a b
The transformed version works well with our flattening logic where it will miss the former completely.
https://github.com/prettier/prettier/blob/d2bcaea121d244a00100e5c31e2e8ca9fe5f1ced/src/language-js/parse/postprocess/index.js#L207-L225