refactor(compiler-vapor): skip unnecessary attribute quoting in templates #13673
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for vue-sfc-playground failed. Why did it fail? →
|
| needsQuoting = /[\s>]|^["'=]/.test(value) | ||
|
|
||
| if (needsQuoting) { | ||
| const encoded = value.replace(/"/g, '"') |
There was a problem hiding this comment.
This fixes value='"foo"' from being turned into value=""foo"", I'm not sure if this is the preferrable way to do it, but I've looked into how Vue compiler deals with entities and it seems that we only ever do decoding (which seems bad, e.g. if you have a static element displaying HTML code)
| // https://html.spec.whatwg.org/multipage/introduction.html#intro-early-example | ||
| needsQuoting = /[\s>]|^["'=]/.test(value) |
There was a problem hiding this comment.
I've noticed that the browser diverges slightly from the specification here, the backticks and open angled bracket aren't something browser parsers cares about, and single/double quotes only matters if it's at the start of the value.
This can be tested on the console:
template = document.createElement('template');
template.innerHTML = '<div data-backticks=`good data-end-quote=good" data-open-bracket=<good></div>';
template.innerHTML;Let me know if we should just align to what the specification says, or to add a note that we're diverging and matching the browser.
There was a problem hiding this comment.
When you say "the browser" have you tested that that works in ALL browsers? If not then it would be better to follow the spec to avoid breaking things.
There was a problem hiding this comment.
yes, works fine in Gecko, WebKit and Blink. I can't test in Ladybird and Servo at the moment but I'd be surprised if it doesn't. this seems like one of those implementation-level quirk that ends up getting reimplemented across browsers, either that or this quirk is actually documented elsewhere and not on the page I referenced
This PR alters the template generation in Vapor compiler to omit unnecessary quoting of attribute values and inclusion of whitespace between attributes, which when combined with #13667 can save a decent amount of bytes
<div id="foo" class="bar"></div><div id=foo class=bar></div><div id="foo>bar" class="has whitespace"></div><div id="foo>bar"class="has whitespace"></div>