feat(format/html): implement suppression comments #5356

dyc3 · 2025-03-14T20:54:56Z

Summary

This reverts #4056 to make HTML comments actual comment trivia. It also attempts to fix the formatting for comments as well, trying to preserve whitespace in the same places that prettier does.

This PR is also the result of a cascading clusterfuck of issues that appeared as a result of making this change, so it may seem a little scatterbrained, and a little difficult to review.

This PR also doesn't attempt to handle or reimplement prettier-ignore-attribute comments in any way. Example from prettier docs:

<!-- prettier-ignore -->
<div         class="x"       >hello world</div            >

<!-- prettier-ignore-attribute -->
<div
  (mousedown)="       onStart    (    )         "
  (mouseup)="         onEnd      (    )         "
></div>

<!-- prettier-ignore-attribute (mouseup) -->
<div
  (mousedown)="onStart()"
  (mouseup)="         onEnd      (    )         "
></div>

The reason these exist is because you can't put comments inside the <> of HTML tags.

Test Plan

CI should pass. Added new tests, and updated snapshots.

I've temporarily disabled specs::prettier::html::comments::surrounding_empty_line_html because I'm actually loosing my mind and its the only one failing debug assertions now. 🫠 I'll fix it later.

github-actions · 2025-03-14T20:59:10Z

Parser conformance results on

js/262

Test result	`main` count	This PR count	Difference
Total	50627	50627	0
Passed	49357	49357	0
Failed	1270	1270	0
Panics	0	0	0
Coverage	97.49%	97.49%	0.00%

jsx/babel

Test result	`main` count	This PR count	Difference
Total	40	40	0
Passed	37	37	0
Failed	3	3	0
Panics	0	0	0
Coverage	92.50%	92.50%	0.00%

symbols/microsoft

Test result	`main` count	This PR count	Difference
Total	6706	6706	0
Passed	2245	2245	0
Failed	4461	4461	0
Panics	0	0	0
Coverage	33.48%	33.48%	0.00%

ts/babel

Test result	`main` count	This PR count	Difference
Total	822	822	0
Passed	731	731	0
Failed	91	91	0
Panics	0	0	0
Coverage	88.93%	88.93%	0.00%

ts/microsoft

Test result	`main` count	This PR count	Difference
Total	18785	18785	0
Passed	14413	14413	0
Failed	4372	4372	0
Panics	0	0	0
Coverage	76.73%	76.73%	0.00%

codspeed-hq · 2025-03-14T21:33:06Z

CodSpeed Performance Report

Merging #5356 will not alter performance

_{Comparing html-suppression-comments (31c0b15) with main (0cc38f5)¹}

Summary

✅ 129 untouched benchmarks

No successful run was found on main (5f328e1) during the generation of this report, so 0cc38f5 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

dyc3 · 2025-05-18T22:26:44Z

A quick update on the status of this PR: It's like 90% done, just gotta deal with the other 90%.

I say that unironically though. Every edge case I fix seems to result in some other breakage elsewhere. I have legitimately spent 100+ hours debugging various breakages in this PR, mostly because of the differences between "trivia on tokens" vs "trivia associated with nodes based on impl CommentStyle for HtmlCommentStyle", and because of how the HtmlElementList formatter opts out of the automatic formatting of children for the sake of whitespace sensitivity, and I'm quite frustrated with it. For that reason, I do not want this to end up in 2.0. It can wait until 2.1.

ematipico · 2025-05-29T16:13:01Z

Totally reasonable! I wished you didn't spend soooo many hours! We can take a break for a bit, or I can take it over for you.

Thank you for the efforts!

dyc3 · 2025-05-29T16:17:08Z

I've actually found the root cause to be that the comment formatters in biome_formatter that are shared between languages actually has a bunch of JS specific logic that can't apply to HTML comment formatting. If that can be resolved, (either by html having its own comment formatters, or removing the js logic from the generic one), I could be convinced to include this in 2.0.

ematipico · 2025-05-29T17:24:52Z

That's weird. CSS, JSON and Graphql use the same infrastructure. I'll look into this when I have time

…into html-suppression-comments

dyc3 · 2025-06-22T19:39:08Z

Ok, so the current regression is complex because it's a hitting the reformatting assertion. There's multiple layers to this.

In short, this prettier case shows the regression: crates/biome_html_formatter/tests/specs/prettier/html/prettier_ignore/cases.html, specifically:

<span></span><!--prettier-ignore--><span></span>

(note that this internally gets turned into a biome-ignore comment).

To run this case:

cargo test -p biome_html_formatter --test prettier_tests -- cases_html

The intended functionality is that:

The 2nd span has it's formatting suppressed (printed verbatim)
No whitespace should be added around the comment (spaces and newlines) because that changes the semantics of the code.

1st format

When this code is formatted, it becomes this:

<span></span><!--prettier-ignore-->
<span></span>

How we get here is a a little convoluted. Firstly, the CommentTextPosition for this comment is correctly detected as SameLine. By default, this would end up being a trailing comment to the first span, but we override it to be a leading comment for the second span here, in impl CommentStyle for HtmlCommentStyle:

biome/crates/biome_html_formatter/src/comments.rs

Lines 78 to 82 in c89e041

    
           if let Some(following_node) = comment.following_node() { 
        
               if comment.text_position().is_same_line() { 
        
                   return CommentPlacement::leading(following_node.clone(), comment); 
        
               } 
        
           }

This leads to the comment getting formatted as a leading comment. This is the exact line that emits the erroneous hard break in FormatLeadingComments: (Line 96)

biome/crates/biome_formatter/src/trivia.rs

Lines 95 to 98 in c89e041

    
           CommentKind::Line => match comment.lines_after() { 
        
               0 | 1 => write!(f, [hard_line_break()])?, 
        
               _ => write!(f, [empty_line()])?, 
        
           },

2nd format

When the code is reformatted, we get a different result.

<span></span> <!--prettier-ignore-->
<span></span>

This time, a space gets added in front of the comment. This is also no good because whitespace sensitivity.

The comment correctly gets detected as CommentTextPosition::EndOfLine. This causes the comment to not become a leading comment of the 2nd span, and actually causes the 1st span to get suppressed.

In FormatTrailingComments, the reason this space gets added is because of the should_nestle logic. The concept of "nestle" does not apply to HTML comments -- it's related to JS doc comments. The exact place where the space gets added is here:

biome/crates/biome_formatter/src/trivia.rs

Lines 190 to 191 in c89e041

    
           let content = 
        
               format_with(|f| write!(f, [maybe_space(!should_nestle), format_comment]));

(because should_nestle is always false and makes the space always get emitted).

The fix?

Naturally, the fix for this would be for HTML to have it's own comment trivia formatters that don't add whitespace. This would make sense because HTML needs fine control over the whitespace that is emitted (again, because whitespace sensitivity).

In fact, we aggressively opt out of much of the formatter's defaults when it comes to formatting HtmlElementList, and its why this snippet (in the same test file!) does not have the same problem:

123<!--prettier-ignore-->456

Normally, we would opt out of biome_formatter's FormatLeadingComments and FormatTrailingComments by re implementing them in the language's trait FormatNodeRule (in their respective functions):

biome/crates/biome_html_formatter/src/lib.rs

Lines 194 to 196 in c89e041

    
               fn fmt_leading_comments(&self, node: &N, f: &mut HtmlFormatter) -> FormatResult<()> { 
        
                   format_leading_comments(node.syntax()).fmt(f) 
        
               }

However, this does not work. This function will never get invoked if the node that is being formatted is suppressed. Instead, it appears that FormatVerbatimNode will not use the language's preferred comment formatters, but will instead use the default implementations, which leads to the previously described behavior.

MichaReiser · 2025-06-25T07:38:38Z

It may make sense to have language-specific format rules for comments with a shared implementation in biome_formatter for languages that all use the same rules.

E.g. we (ruff, python formatter) had to customize the comment placement logic too.

ematipico · 2025-07-02T14:53:08Z

Thank you @MichaReiser , do you have some Ruff code you can share with us, so we can check how you guys fixed the problem?

MichaReiser · 2025-07-07T07:40:35Z

It probably makes sense to see how prettier handles HTML comments. All we did is to move the comment formatting into our language implementation instead of having a generic one. See https://github.com/astral-sh/ruff/blob/9ae698fe30cf3526f0e7ae237b800b3ed19a819f/crates/ruff_python_formatter/src/comments/format.rs#L14-L636

dyc3 · 2025-07-07T12:25:57Z

That makes sense, that's essentially the conclusion I came to last time I worked on this

changeset-bot · 2025-07-07T18:52:28Z

⚠️ No Changeset found

Latest commit: 31c0b15

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

dyc3 · 2025-07-07T19:10:04Z

~~I've fixed the merge conflicts~~ (there have been more since I made this comment) -- @ematipico could you take a look at this again when you get a chance? It's somewhat blocking me from making more progress on the HTML formatter.

…ents

dyc3 · 2025-07-31T18:21:50Z

crates/biome_html_formatter/tests/specs/html/comments/after-text.html.snap

+```html
+Foo
+
+<!-- Bar -->```


This snapshot looks a little weird because it's not adding an extra newline at the end. Not sure if it's worth fixing in this PR.

We can fix it later, I think, after all, the HTML parser and formatter are still experimental. We can follow up, I think

crates/biome_html_formatter/tests/specs/prettier/html/interpolation/example.html.snap

crates/biome_rowan/src/syntax/element.rs

dyc3 · 2025-07-31T18:52:06Z

I believe this is ready to go.

ematipico

Let's go :)

github-actions bot added A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-HTML Language: HTML labels Mar 14, 2025

dyc3 force-pushed the html-suppression-comments branch from 9700f3a to 89290e1 Compare March 14, 2025 22:05

dyc3 force-pushed the html-suppression-comments branch from 89290e1 to 5c441aa Compare April 7, 2025 13:33

github-actions bot added the A-Core Area: core label Apr 7, 2025

dyc3 force-pushed the html-suppression-comments branch from 5c441aa to 100dcd5 Compare April 8, 2025 15:58

dyc3 mentioned this pull request Apr 15, 2025

fix(parse/html): handle unclosed elements more gracefully #5063

Open

dyc3 force-pushed the html-suppression-comments branch 3 times, most recently from f50bbb8 to deaaf94 Compare April 15, 2025 21:40

dyc3 force-pushed the html-suppression-comments branch from deaaf94 to 5d597a8 Compare May 4, 2025 16:31

dyc3 force-pushed the html-suppression-comments branch from 5d597a8 to 9bfa5fa Compare May 18, 2025 22:19

dyc3 force-pushed the html-suppression-comments branch 2 times, most recently from 7d57192 to 4ea4ad2 Compare May 29, 2025 16:11

dyc3 force-pushed the html-suppression-comments branch 2 times, most recently from 7794594 to 29d9f26 Compare May 29, 2025 16:50

dyc3 added 2 commits June 10, 2025 10:44

feat(format/html): implement suppression comments

913bc9a

feat(format/html): implement suppression comments

29e6546

ematipico force-pushed the html-suppression-comments branch from 29d9f26 to 29e6546 Compare June 12, 2025 14:08

ematipico and others added 3 commits June 12, 2025 15:46

fix HTML category

291a69a

Merge branch 'html-suppression-comments' of github.com:biomejs/biome …

64bbb6d

…into html-suppression-comments

fix changes that were accidentally reverted

c89e041

Merge branch 'main' into html-suppression-comments

3bcb0af

dyc3 added 2 commits July 7, 2025 14:56

fix: typo

bde9bdf

fix: regressions from merge

11479f1

dyc3 mentioned this pull request Jul 24, 2025

feat(parser/html): text expressions in attributes #6907

Merged

1 task

ematipico mentioned this pull request Jul 25, 2025

refactor(formatter): specialised verbatim formatting #7002

Merged

ematipico closed this in #7002 Jul 25, 2025

ematipico reopened this Jul 28, 2025

ematipico mentioned this pull request Jul 28, 2025

refactor(format): use specialised tokens #7052

Merged

ematipico added 4 commits July 30, 2025 08:34

Merge remote-tracking branch 'origin/main' into html-suppression-comm…

9ff83f4

…ents

chore: fix merges and re-adapt code

9d2b55b

chore: fix reformat issues

15427ad

clippy

0329286

dyc3 commented Jul 31, 2025

View reviewed changes

dyc3 added 2 commits July 31, 2025 14:31

chore: revert a debugging change

95e5e8f

fix: minor regression

31c0b15

dyc3 marked this pull request as ready for review July 31, 2025 18:51

github-actions bot removed the A-Core Area: core label Jul 31, 2025

dyc3 requested a review from ematipico July 31, 2025 18:52

ematipico approved these changes Jul 31, 2025

View reviewed changes

ematipico merged commit 2e74648 into main Jul 31, 2025
30 checks passed

ematipico deleted the html-suppression-comments branch July 31, 2025 20:53

Uh oh!

feat(format/html): implement suppression comments #5356

feat(format/html): implement suppression comments #5356

Uh oh!

Conversation

dyc3 commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Parser conformance results on

js/262

jsx/babel

symbols/microsoft

ts/babel

ts/microsoft

Uh oh!

codspeed-hq bot commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #5356 will not alter performance

Summary

Footnotes

Uh oh!

dyc3 commented May 18, 2025

Uh oh!

ematipico commented May 29, 2025

Uh oh!

dyc3 commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ematipico commented May 29, 2025

Uh oh!

dyc3 commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1st format

2nd format

The fix?

Uh oh!

MichaReiser commented Jun 25, 2025

Uh oh!

ematipico commented Jul 2, 2025

Uh oh!

MichaReiser commented Jul 7, 2025

Uh oh!

dyc3 commented Jul 7, 2025

Uh oh!

changeset-bot bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

dyc3 commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dyc3 Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ematipico Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dyc3 commented Jul 31, 2025

Uh oh!

ematipico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dyc3 commented Mar 14, 2025 •

edited

Loading

github-actions bot commented Mar 14, 2025 •

edited

Loading

codspeed-hq bot commented Mar 14, 2025 •

edited

Loading

dyc3 commented May 29, 2025 •

edited

Loading

dyc3 commented Jun 22, 2025 •

edited

Loading

changeset-bot bot commented Jul 7, 2025 •

edited

Loading

dyc3 commented Jul 7, 2025 •

edited

Loading

dyc3 Jul 31, 2025 •

edited

Loading