This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
feat(rome_js_formatter): object pattern formatting #2729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with
|
| Latest commit: |
796332b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://952d1605.tools-8rn.pages.dev |
| Branch Preview URL: | https://feature-object-pattern-forma.tools-8rn.pages.dev |
denbezrukov
reviewed
Jun 20, 2022
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
8b68a73
to
d10fcf5
Compare
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
d10fcf5
to
984337f
Compare
ematipico
commented
Jun 21, 2022
MichaReiser
reviewed
Jun 21, 2022
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
b0d27d0
to
e76dc00
Compare
MichaReiser
approved these changes
Jun 22, 2022
denbezrukov
reviewed
Jun 22, 2022
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
6cb4386
to
b168b2e
Compare
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
6ffab94
to
8ef485a
Compare
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
8ef485a
to
bc1479a
Compare
MichaReiser
reviewed
Jun 22, 2022
| // 3. we compute the layout | ||
| // 4. we write the left node inside the main buffer based on the layout | ||
| let mut buffer = VecBuffer::new(f.state_mut()); | ||
| let is_left_short = self.write_left(&mut buffer)?; |
There was a problem hiding this comment.
You can do
write!(buffer, [format_with(|f| self.write_left(f)])?;
to get a formatter for a buffer
| let layout = self.layout(is_left_short)?; | ||
|
|
||
| let formatted_element = buffer.into_element(); | ||
|
|
||
| if layout == AssignmentLikeLayout::BreakLeftHandSide { |
There was a problem hiding this comment.
This works fine but has the downside (isn't specific to here, we have this all over the place), that the implementation needs to copy all elements that have been written to the VecBuffer. I'll spend a few minutes looking into a InternBuffer that returns an interned element.
There was a problem hiding this comment.
I just copied what we already do in other places :D
ematipico
force-pushed
the
feature/object-pattern-formatting
branch
from
bc1479a
to
796332b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR advances works around "assignments like", by partially implementing #2424
Here's the main works done:
const { b } = a;andconst a = { b }using the same function, this is because in their AST they are treated with the same AST nodes. Rome used two different nodes:JsObjectAssignmentPatternandJsObjectBindingPattern. In this PR I created a new union calledJsObjectPatternLikewhere we apply the same formatting for both patterns;BreakLeftHandSideandChainTailArrowChain. The first had required some changes inside the object pattern formatting, which are included in this PR, the second one won't have the desidered changes because of how things are built.WhenTurns out the implementation was easier than expected, and it is now implemented!ChainTailArrowChain, prettier signals this layout to another function, which reads it and applies another layout. Here's the code that reads the layout. I'd prefer to figure this out later, outside of this PR (fortunately we don't break any existing code)JsAnyAssignmentLike, calledRightAssignmentLike. This was needed because the right hand side of an assignment like can be anJsAnyExpressionorJsAnyAssignmentPattern;BreakLeftHandSide, because the rest of the checks are done on other types of nodes (type aliases and type parameters), which are not covered in this PR.Test Plan
I created new test cases that should cover the new layouts