Skip to content

Commit

Permalink
fix(compiler-dom): restrict createStaticVNode usage with option eleme…
Browse files Browse the repository at this point in the history
…nts (vuejs#10846)

close vuejs#6568
close vuejs#7434
  • Loading branch information
skirtles-code committed May 1, 2024
1 parent c9c9dff commit 0e3d617
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`stringify static html > should bail for <option> elements with number values 1`] = `
"const { createElementVNode: _createElementVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
const _hoisted_1 = /*#__PURE__*/_createElementVNode("select", null, [
/*#__PURE__*/_createElementVNode("option", { value: 1 }),
/*#__PURE__*/_createElementVNode("option", { value: 1 }),
/*#__PURE__*/_createElementVNode("option", { value: 1 }),
/*#__PURE__*/_createElementVNode("option", { value: 1 }),
/*#__PURE__*/_createElementVNode("option", { value: 1 })
], -1 /* HOISTED */)
const _hoisted_2 = [
_hoisted_1
]
return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, _hoisted_2))
}"
`;
exports[`stringify static html > should bail on bindings that are hoisted but not stringifiable 1`] = `
"const { createElementVNode: _createElementVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
Expand All @@ -20,6 +39,19 @@ return function render(_ctx, _cache) {
}"
`;
exports[`stringify static html > should work for <option> elements with string values 1`] = `
"const { createElementVNode: _createElementVNode, createStaticVNode: _createStaticVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
const _hoisted_1 = /*#__PURE__*/_createStaticVNode("<select><option value=\\"1\\"></option><option value=\\"1\\"></option><option value=\\"1\\"></option><option value=\\"1\\"></option><option value=\\"1\\"></option></select>", 1)
const _hoisted_2 = [
_hoisted_1
]
return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, _hoisted_2))
}"
`;
exports[`stringify static html > should work with bindings that are non-static but stringifiable 1`] = `
"const { createElementVNode: _createElementVNode, createStaticVNode: _createStaticVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
Expand Down
47 changes: 47 additions & 0 deletions packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,51 @@ describe('stringify static html', () => {
expect(code).toMatch(`<code>text1</code>`)
expect(code).toMatchSnapshot()
})

test('should work for <option> elements with string values', () => {
const { ast, code } = compileWithStringify(
`<div><select>${repeat(
`<option value="1" />`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT,
)}</select></div>`,
)
// should be optimized now
expect(ast.hoists).toMatchObject([
{
type: NodeTypes.JS_CALL_EXPRESSION,
callee: CREATE_STATIC,
arguments: [
JSON.stringify(
`<select>${repeat(
`<option value="1"></option>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT,
)}</select>`,
),
'1',
],
},
{
type: NodeTypes.JS_ARRAY_EXPRESSION,
},
])
expect(code).toMatchSnapshot()
})

test('should bail for <option> elements with number values', () => {
const { ast, code } = compileWithStringify(
`<div><select>${repeat(
`<option :value="1" />`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT,
)}</select></div>`,
)
expect(ast.hoists).toMatchObject([
{
type: NodeTypes.VNODE_CALL,
},
{
type: NodeTypes.JS_ARRAY_EXPRESSION,
},
])
expect(code).toMatchSnapshot()
})
})
12 changes: 12 additions & 0 deletions packages/compiler-dom/src/transforms/stringifyStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
type TextCallNode,
type TransformContext,
createCallExpression,
isStaticArgOf,
} from '@vue/compiler-core'
import {
escapeHtml,
Expand Down Expand Up @@ -200,6 +201,7 @@ function analyzeNode(node: StringifiableNode): [number, number] | false {
// probably only need to check for most common case
// i.e. non-phrasing-content tags inside `<p>`
function walk(node: ElementNode): boolean {
const isOptionTag = node.tag === 'option' && node.ns === Namespaces.HTML
for (let i = 0; i < node.props.length; i++) {
const p = node.props[i]
// bail on non-attr bindings
Expand All @@ -225,6 +227,16 @@ function analyzeNode(node: StringifiableNode): [number, number] | false {
) {
return bail()
}
// <option :value="1"> cannot be safely stringified
if (
isOptionTag &&
isStaticArgOf(p.arg, 'value') &&
p.exp &&
p.exp.ast &&
p.exp.ast.type !== 'StringLiteral'
) {
return bail()
}
}
}
for (let i = 0; i < node.children.length; i++) {
Expand Down

0 comments on commit 0e3d617

Please sign in to comment.