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

fix(rome_js_formatter): Single-line comment below a JSX prop triggers… #3641

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

denbezrukov
Copy link
Contributor

@denbezrukov denbezrukov commented Nov 10, 2022

@netlify
Copy link

netlify bot commented Nov 10, 2022

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit 295ff31
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6371ec6fcefa8d000ae1778c

@@ -25,7 +25,6 @@ impl FormatNodeRule<JsxClosingElement> for FormatJsxClosingElement {
l_angle_token.format(),
slash_token.format(),
name.format(),
line_suffix_boundary(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is safe or is it handled inside of the name formatting?. What happens for:

<div>content</div // comment
> 

It seems that prettier has some explicit comment handling that we do not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.
added a test case.

Copy link
Contributor

@MichaReiser MichaReiser Nov 10, 2022

Choose a reason for hiding this comment

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

Did you push the latest changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did,

I reverted changes for the file and added test case

let b = (
	<section>
		<div>
			aVeryLongCOntentThat
		</div // comment
	>
	</section>
);

@@ -45,7 +45,6 @@ impl Format<JsFormatContext> for JsxAnyOpeningElement {
l_angle_token.format(),
name.format(),
type_arguments.format(),
line_suffix_boundary(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

... the main reason this is safe because the Inline layout guarantees that neither the name, type arguments have any comments and arguments is obviously empty :)

@@ -45,7 +45,6 @@ impl Format<JsFormatContext> for JsxAnyOpeningElement {
l_angle_token.format(),
name.format(),
type_arguments.format(),
line_suffix_boundary(),
Copy link
Contributor

Choose a reason for hiding this comment

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

... the main reason this is safe because the Inline layout guarantees that neither the name, type arguments have any comments and arguments is obviously empty :)

@MichaReiser MichaReiser added this to the 11.0.0 milestone Nov 10, 2022
@MichaReiser MichaReiser merged commit e630a10 into rome:main Nov 14, 2022
jeysal added a commit to jeysal/rometools that referenced this pull request Nov 14, 2022
* upstream/main: (45 commits)
  website(docs): set `color-scheme` on the root element (rome#3721)
  feat(rome_analyze): add a warning for unused suppression comments (rome#3718)
  feat(rome_js_analyze): Implement prefer-numeric-literals lint (rome#3558)
  feat(rome_js_formatter): jestEach template literals rome#3308 (rome#3582)
  doc(website): Add context about Romes philosophy (rome#3714)
  fix(rome_js_formatter): Single-line comment below a JSX prop triggers… (rome#3641)
  test(rome_js_formatter): update prettier tests (rome#3684)
  fix(rome_js_parser): improve await handling in non-async context (rome#3573)
  fix(rome_js_parser): improve yield parsing in non generator function (rome#3622)
  More playground polish
  Fix backgrounds
  Fix height
  Align docs.rome.tools with rome.tools
  Reenable compression
  Add missing width
  website(docs): More playground IDE features (rome#3711)
  fix(rome_js_formatter): new expression attribute (rome#3686)
  docs(website): added checkbox to toggle linter in playground (rome#3699)
  website(docs): More website tweaks (rome#3707)
  website(docs): Add default layout property (rome#3705)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Single-line comment below a JSX prop triggers different formatting
2 participants