Skip to content

refactor(compiler-vapor): skip unnecessary attribute quoting in templates #13673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: minor
Choose a base branch
from
+121 −13
Open

refactor(compiler-vapor): skip unnecessary attribute quoting in templates #13673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
083c084
refactor(compiler-vapor): skip unnecessary attribute quoting in templ…
mary-ext Jul 21, 2025
2fe9c1c
refactor(compiler-vapor): skip unnecessary whitespace between attributes
mary-ext Jul 21, 2025
0b5b4ca
fix(compiler-vapor): unset needsQuoting if there is no value
mary-ext Jul 21, 2025
3e90f4e
fix(compiler-vapor): encode quotes back
mary-ext Jul 21, 2025
ba9cd1c
chore(compiler-vapor): add runtime test for attribute parsing behavior
mary-ext Jul 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/compiler-vapor/__tests__/__snapshots__/compile.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export function render(_ctx) {

exports[`compile > directives > v-pre > basic 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div :id=\\"foo\\"><Comp></Comp>{{ bar }}</div>", true)
const t0 = _template("<div :id=foo><Comp></Comp>{{ bar }}</div>", true)

export function render(_ctx, $props, $emit, $attrs, $slots) {
const n0 = t0()
Expand All @@ -150,7 +150,7 @@ export function render(_ctx, $props, $emit, $attrs, $slots) {

exports[`compile > directives > v-pre > should not affect siblings after it 1`] = `
"import { resolveComponent as _resolveComponent, setInsertionState as _setInsertionState, createComponentWithFallback as _createComponentWithFallback, child as _child, setProp as _setProp, toDisplayString as _toDisplayString, setText as _setText, renderEffect as _renderEffect, template as _template } from 'vue';
const t0 = _template("<div :id=\\"foo\\"><Comp></Comp>{{ bar }}</div>")
const t0 = _template("<div :id=foo><Comp></Comp>{{ bar }}</div>")
const t1 = _template("<div> </div>")

export function render(_ctx, $props, $emit, $attrs, $slots) {
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-vapor/__tests__/compile.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('compile', () => {

expect(code).toMatchSnapshot()
expect(code).contains(
JSON.stringify('<div :id="foo"><Comp></Comp>{{ bar }}</div>'),
JSON.stringify('<div :id=foo><Comp></Comp>{{ bar }}</div>'),
)
expect(code).not.contains('effect')
})
Expand Down
34 changes: 32 additions & 2 deletions packages/compiler-vapor/__tests__/transforms/__snapshots__/transformElement.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export function render(_ctx) {

exports[`compiler: element transform > props + children 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div id=\\"foo\\"><span></span></div>", true)
const t0 = _template("<div id=foo><span></span></div>", true)

export function render(_ctx) {
const n0 = t0()
Expand Down Expand Up @@ -399,7 +399,37 @@ export function render(_ctx) {

exports[`compiler: element transform > static props 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div id=\\"foo\\" class=\\"bar\\"></div>", true)
const t0 = _template("<div id=foo class=bar></div>", true)

export function render(_ctx) {
const n0 = t0()
return n0
}"
`;

exports[`compiler: element transform > static props mixed quoting 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div title=\\"has whitespace\\"inert data-targets=\\"foo>bar\\"></div>", true)

export function render(_ctx) {
const n0 = t0()
return n0
}"
`;

exports[`compiler: element transform > static props quoted 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div title=\\"has whitespace\\"alt=\\"&#34;contains quotes&#34;\\"data-targets=\\"foo>bar\\"></div>", true)

export function render(_ctx) {
const n0 = t0()
return n0
}"
`;

exports[`compiler: element transform > static props unquoted 1`] = `
"import { template as _template } from 'vue';
const t0 = _template("<div id=foo class=bar title=foo\\"=bar></div>", true)

export function render(_ctx) {
const n0 = t0()
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-vapor/__tests__/transforms/__snapshots__/vBind.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ export function render(_ctx) {

exports[`compiler v-bind > with constant value 1`] = `
"import { setProp as _setProp, template as _template } from 'vue';
const t0 = _template("<div f=\\"foo1\\" h=\\"1\\"></div>", true)
const t0 = _template("<div f=foo1 h=1></div>", true)

export function render(_ctx, $props, $emit, $attrs, $slots) {
const n0 = t0()
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-vapor/__tests__/transforms/__snapshots__/vModel.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function render(_ctx) {

exports[`compiler: vModel transform > should support input (checkbox) 1`] = `
"import { applyCheckboxModel as _applyCheckboxModel, template as _template } from 'vue';
const t0 = _template("<input type=\\"checkbox\\">", true)
const t0 = _template("<input type=checkbox>", true)

export function render(_ctx) {
const n0 = t0()
Expand All @@ -138,7 +138,7 @@ export function render(_ctx) {

exports[`compiler: vModel transform > should support input (radio) 1`] = `
"import { applyRadioModel as _applyRadioModel, template as _template } from 'vue';
const t0 = _template("<input type=\\"radio\\">", true)
const t0 = _template("<input type=radio>", true)

export function render(_ctx) {
const n0 = t0()
Expand All @@ -149,7 +149,7 @@ export function render(_ctx) {

exports[`compiler: vModel transform > should support input (text) 1`] = `
"import { applyTextModel as _applyTextModel, template as _template } from 'vue';
const t0 = _template("<input type=\\"text\\">", true)
const t0 = _template("<input type=text>", true)

export function render(_ctx) {
const n0 = t0()
Expand Down
42 changes: 40 additions & 2 deletions packages/compiler-vapor/__tests__/transforms/transformElement.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,45 @@ describe('compiler: element transform', () => {
`<div id="foo" class="bar" />`,
)

const template = '<div id="foo" class="bar"></div>'
const template = '<div id=foo class=bar></div>'
expect(code).toMatchSnapshot()
expect(code).contains(JSON.stringify(template))
expect(ir.template).toMatchObject([template])
expect(ir.block.effect).lengthOf(0)
})

test('static props unquoted', () => {
const { code, ir } = compileWithElementTransform(
`<div id="foo" class="bar" title='foo"=bar' />`,
)

const template = '<div id=foo class=bar title=foo"=bar></div>'
expect(code).toMatchSnapshot()
expect(code).contains(JSON.stringify(template))
expect(ir.template).toMatchObject([template])
expect(ir.block.effect).lengthOf(0)
})

test('static props quoted', () => {
const { code, ir } = compileWithElementTransform(
`<div title="has whitespace" alt='"contains quotes"' data-targets="foo>bar" />`,
)

const template =
'<div title="has whitespace"alt="&#34;contains quotes&#34;"data-targets="foo>bar"></div>'
expect(code).toMatchSnapshot()
expect(code).contains(JSON.stringify(template))
expect(ir.template).toMatchObject([template])
expect(ir.block.effect).lengthOf(0)
})

test('static props mixed quoting', () => {
const { code, ir } = compileWithElementTransform(
`<div title="has whitespace" inert data-targets="foo>bar" />`,
)

const template =
'<div title="has whitespace"inert data-targets="foo>bar"></div>'
expect(code).toMatchSnapshot()
expect(code).contains(JSON.stringify(template))
expect(ir.template).toMatchObject([template])
Expand All @@ -588,7 +626,7 @@ describe('compiler: element transform', () => {
`<div id="foo"><span/></div>`,
)

const template = '<div id="foo"><span></span></div>'
const template = '<div id=foo><span></span></div>'
expect(code).toMatchSnapshot()
expect(code).contains(JSON.stringify(template))
expect(ir.template).toMatchObject([template])
Expand Down
23 changes: 21 additions & 2 deletions packages/compiler-vapor/src/transforms/transformElement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,30 @@ function transformNativeElement(
getEffectIndex,
)
} else {
let needsQuoting = false

for (const prop of propsResult[1]) {
const { key, values } = prop
if (key.isStatic && values.length === 1 && values[0].isStatic) {
template += ` ${key.content}`
if (values[0].content) template += `="${values[0].content}"`
const value = values[0].content

if (!needsQuoting) template += ` `
template += key.content

if (value) {
// https://html.spec.whatwg.org/multipage/introduction.html#intro-early-example
needsQuoting = /[\s>]|^["'=]/.test(value)
Comment on lines +235 to +236
Copy link
Author

@mary-ext mary-ext Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Author

@mary-ext mary-ext Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

This behavior is not directly tested by WPT, but they pull some tests from html5lib which has (among the tests not included in WPT at this time) tests for these quirks, which seem to indicate that it's invalid but tokenized properly (and therefore likely to gracefully be accepted by browsers as-is). https://github.com/html5lib/html5lib-tests/blob/a9f44960a9fedf265093d22b2aa3c7ca123727b9/tokenizer/test3.test#L10461-L10466

While it's not as convincing as it could be given the fact it's not pulled by WPT, Servo does uses them


if (needsQuoting) {
const encoded = value.replace(/"/g, '&#34;')
Copy link
Author

@mary-ext mary-ext Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a partial fix for value='"foo"' being turned into value=""foo"", I don't think this is the way to do it though, especially since this doesn't escape & as well.

I've looked into how Vue compiler deals with entities and it seems that we only ever do decoding (which makes sense for the original compiler, but it seems bad for Vapor where we generate HTML templates, we don't want static HTML code examples to be parsed as anything but text)


template += `="${encoded}"`
} else {
template += `=${value}`
}
} else {
needsQuoting = false
}
} else {
dynamicProps.push(key.content)
context.registerEffect(
Expand Down
21 changes: 21 additions & 0 deletions packages/runtime-vapor/__tests__/dom/template.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,25 @@ describe('api: template', () => {
expect(nthChild(root, 2)).toBe(root.childNodes[2])
expect(next(b)).toBe(root.childNodes[2])
})

test('attribute quote omission', () => {
{
const t = template('<div id=foo class=bar alt=`<="foo></div>')
const root = t() as HTMLElement

expect(root.attributes).toHaveLength(3)
expect(root.getAttribute('id')).toBe('foo')
expect(root.getAttribute('class')).toBe('bar')
expect(root.getAttribute('alt')).toBe('`<="foo')
}

{
const t = template('<div id="foo>bar"class="has whitespace"></div>')
const root = t() as HTMLElement

expect(root.attributes).toHaveLength(2)
expect(root.getAttribute('id')).toBe('foo>bar')
expect(root.getAttribute('class')).toBe('has whitespace')
}
})
})