Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_formatter): EcmaScript @decorators #4252 #4442

Merged
merged 3 commits into from May 8, 2023
Merged

feat(rome_js_formatter): EcmaScript @decorators #4252 #4442

merged 3 commits into from May 8, 2023

Conversation

denbezrukov
Copy link
Contributor

@denbezrukov denbezrukov commented May 7, 2023 •
edited

Summary

This PR implements the decorator formatting.

https://github.com/prettier/prettier/blob/main/src/language-js/print/decorators.js

@dec1 export @dec2 class My {}

class Foo {
	@dec property;
	@dec
	method() {}
}

class Foo extends (@deco class Foo {}){}

@dec1
@dec2
export class My {}

class Foo {
	@dec property;
	@dec
	method() {}
}

class Foo extends (
	@deco
	class Foo {}
) {}

Test Plan

cargo test -p rome_js_formatter

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented May 7, 2023 •
edited

✅ Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 2eff405
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/64591b582fb5a7000881044e
😎 Deploy Preview https://deploy-preview-4442--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the A-Formatter Area: formatter label May 7, 2023
@denbezrukov denbezrukov marked this pull request as ready for review May 8, 2023 07:01
@denbezrukov denbezrukov requested a review from ematipico May 8, 2023 07:01
denbezrukov
denbezrukov commented May 8, 2023
View reviewed changes
crates/rome_js_formatter/src/js/expressions/class_expression.rs
soft_line_break_or_space(),
&FormatClass::from(&node.clone().into()),
]),
soft_line_break_or_space()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/prettier/prettier/blob/3febd6e138ffc8cb3df634dade9c96d8df922650/src/language-js/print/index.js#L93-L101

denbezrukov
denbezrukov commented May 8, 2023
View reviewed changes
crates/rome_js_formatter/src/js/lists/decorator_list.rs Outdated
f.join_with(&soft_line_break_or_space())
.entries(node.iter().formatted())
.finish()?;
// we need to rearrange decorators to be before export if we have decorators before class
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/prettier/prettier/blob/3febd6e138ffc8cb3df634dade9c96d8df922650/src/language-js/print/decorators.js#L37-L67

denbezrukov
denbezrukov commented May 8, 2023
View reviewed changes
crates/rome_js_formatter/src/utils/format_modifiers.rs
let mut join = f.join_nodes_with_soft_line();

// join only decorators here
while let Some(node) = iter.peek() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/prettier/prettier/blob/3febd6e138ffc8cb3df634dade9c96d8df922650/src/language-js/print/decorators.js#L14-L20

denbezrukov
denbezrukov commented May 8, 2023
View reviewed changes
crates/rome_js_formatter/src/utils/assignment_like.rs
@@ -941,6 +941,8 @@ pub(crate) fn should_break_after_operator(
})
}

AnyJsExpression::JsClassExpression(class) => !class.decorators().is_empty(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/prettier/prettier/blob/3febd6e138ffc8cb3df634dade9c96d8df922650/src/language-js/print/assignment.js#L198

denbezrukov
denbezrukov commented May 8, 2023
View reviewed changes
crates/rome_js_formatter/src/js/expressions/class_expression.rs
}

fn needs_parentheses(&self, item: &JsClassExpression) -> bool {
item.needs_parentheses()
!item.decorators().is_empty() || item.needs_parentheses()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/prettier/prettier/blob/3febd6e138ffc8cb3df634dade9c96d8df922650/src/language-js/needs-parens.js#L176

ematipico
ematipico approved these changes May 8, 2023
View reviewed changes
Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! 🏆 Let's ship it!

crates/rome_js_formatter/src/js/lists/decorator_list.rs Outdated Show resolved Hide resolved
@denbezrukov @ematipico
Update crates/rome_js_formatter/src/js/lists/decorator_list.rs
2eff405 
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
@denbezrukov denbezrukov merged commit ff02317 into main May 8, 2023
17 checks passed
@denbezrukov denbezrukov deleted the feat/decorator-format branch May 8, 2023 16:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ematipico ematipico ematipico approved these changes

Assignees
No one assigned
Labels
A-Formatter Area: formatter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@denbezrukov @ematipico