feat(html_parser): add name reference identifier#8827
feat(html_parser): add name reference identifier#8827Netail wants to merge 1 commit intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 0befd10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Performance Changes
Comparing Footnotes
|
WalkthroughThis changeset introduces support for HTML element names that can be reference identifiers rather than just literal tag names. The grammar is updated to define Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
dyc3
left a comment
There was a problem hiding this comment.
There aren't any actual changes to the HTML parser so that it emits the new node.
I think the way this should work is that HtmlReferenceIdentifier should only be emitted if we are in a template language file (.vue, .svelte, etc.)
| node: &HtmlReferenceIdentifier, | ||
| f: &mut HtmlFormatter, | ||
| ) -> FormatResult<()> { | ||
| format_html_verbatim_node(node.syntax()).fmt(f) |
There was a problem hiding this comment.
This needs to be actually formatted. This function marks the tokens as unformatted.
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Add support for the name of an element being a reference identifier |
There was a problem hiding this comment.
nit: should be past tense, should also mention the HTML parser
| && token | ||
| .text_trimmed() | ||
| .eq(token.text_trimmed().to_lowercase_cow().as_ref()) |
There was a problem hiding this comment.
you can check if its all lowercase without the potential allocation. you can .iter().any(...) to check if any char is uppercase.
| let HtmlTagNameFields { value_token } = node.as_fields(); | ||
|
|
||
| if should_lowercase_html_tag(f, node) { | ||
| if should_lowercase_html_tag(f, &node.clone().into()) { |
There was a problem hiding this comment.
I feel like the cloning could be avoided here.
| /// Returns `true` only for canonical HTML tags in pure HTML files (.html). | ||
| /// Component frameworks preserve tag name casing. | ||
| pub(crate) fn should_lowercase_html_tag(f: &HtmlFormatter, tag_name: &HtmlTagName) -> bool { | ||
| pub(crate) fn should_lowercase_html_tag(f: &HtmlFormatter, tag_name: &AnyHtmlTagName) -> bool { |
There was a problem hiding this comment.
I actually don't think this change is necessary, and it would let you avoid the clone I mentioned in my previous comment.
|
Oh, I think this has been covered with #8886 already |
Summary
Add support for the name of an element being a reference identifier (hopefully into the right direction?)
Test Plan
Docs