Skip to content

Commit 29beda7

Browse files
committed
fix(compiler-dom): avoid bailing stringification on setup const bindings
1 parent 4713578 commit 29beda7

File tree

5 files changed

+103
-31
lines changed

5 files changed

+103
-31
lines changed

packages/compiler-core/src/transforms/hoistStatic.ts

+7-25
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,6 @@ function walk(
5252
context: TransformContext,
5353
doNotHoistNode: boolean = false
5454
) {
55-
// Some transforms, e.g. transformAssetUrls from @vue/compiler-sfc, replaces
56-
// static bindings with expressions. These expressions are guaranteed to be
57-
// constant so they are still eligible for hoisting, but they are only
58-
// available at runtime and therefore cannot be evaluated ahead of time.
59-
// This is only a concern for pre-stringification (via transformHoist by
60-
// @vue/compiler-dom), but doing it here allows us to perform only one full
61-
// walk of the AST and allow `stringifyStatic` to stop walking as soon as its
62-
// stringification threshold is met.
63-
let canStringify = true
64-
6555
const { children } = node
6656
const originalCount = children.length
6757
let hoistedCount = 0
@@ -77,9 +67,6 @@ function walk(
7767
? ConstantTypes.NOT_CONSTANT
7868
: getConstantType(child, context)
7969
if (constantType > ConstantTypes.NOT_CONSTANT) {
80-
if (constantType < ConstantTypes.CAN_STRINGIFY) {
81-
canStringify = false
82-
}
8370
if (constantType >= ConstantTypes.CAN_HOIST) {
8471
;(child.codegenNode as VNodeCall).patchFlag =
8572
PatchFlags.HOISTED + (__DEV__ ? ` /* HOISTED */` : ``)
@@ -110,17 +97,12 @@ function walk(
11097
}
11198
}
11299
}
113-
} else if (child.type === NodeTypes.TEXT_CALL) {
114-
const contentType = getConstantType(child.content, context)
115-
if (contentType > 0) {
116-
if (contentType < ConstantTypes.CAN_STRINGIFY) {
117-
canStringify = false
118-
}
119-
if (contentType >= ConstantTypes.CAN_HOIST) {
120-
child.codegenNode = context.hoist(child.codegenNode)
121-
hoistedCount++
122-
}
123-
}
100+
} else if (
101+
child.type === NodeTypes.TEXT_CALL &&
102+
getConstantType(child.content, context) >= ConstantTypes.CAN_HOIST
103+
) {
104+
child.codegenNode = context.hoist(child.codegenNode)
105+
hoistedCount++
124106
}
125107

126108
// walk further
@@ -148,7 +130,7 @@ function walk(
148130
}
149131
}
150132

151-
if (canStringify && hoistedCount && context.transformHoist) {
133+
if (hoistedCount && context.transformHoist) {
152134
context.transformHoist(children, context, node)
153135
}
154136

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`stringify static html should bail on bindings that are hoisted but not stringifiable 1`] = `
4+
"const { createElementVNode: _createElementVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
5+
6+
const _hoisted_1 = /*#__PURE__*/_createElementVNode(\\"div\\", null, [
7+
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
8+
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
9+
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
10+
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
11+
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
12+
/*#__PURE__*/_createElementVNode(\\"img\\", { src: _imports_0_ })
13+
], -1 /* HOISTED */)
14+
const _hoisted_2 = [
15+
_hoisted_1
16+
]
17+
18+
return function render(_ctx, _cache) {
19+
return (_openBlock(), _createElementBlock(\\"div\\", null, _hoisted_2))
20+
}"
21+
`;
22+
23+
exports[`stringify static html should work with bindings that are non-static but stringifiable 1`] = `
24+
"const { createElementVNode: _createElementVNode, createStaticVNode: _createStaticVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
25+
26+
const _hoisted_1 = /*#__PURE__*/_createStaticVNode(\\"<div><span class=\\\\\\"foo\\\\\\">foo</span><span class=\\\\\\"foo\\\\\\">foo</span><span class=\\\\\\"foo\\\\\\">foo</span><span class=\\\\\\"foo\\\\\\">foo</span><span class=\\\\\\"foo\\\\\\">foo</span><img src=\\\\\\"\\" + _imports_0_ + \\"\\\\\\"></div>\\", 1)
27+
const _hoisted_2 = [
28+
_hoisted_1
29+
]
30+
31+
return function render(_ctx, _cache) {
32+
return (_openBlock(), _createElementBlock(\\"div\\", null, _hoisted_2))
33+
}"
34+
`;

packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts

+52-3
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ describe('stringify static html', () => {
181181
])
182182
})
183183

184-
test('should bail on runtime constant v-bind bindings', () => {
185-
const { ast } = compile(
184+
test('should bail on bindings that are hoisted but not stringifiable', () => {
185+
const { ast, code } = compile(
186186
`<div><div>${repeat(
187187
`<span class="foo">foo</span>`,
188188
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
@@ -216,13 +216,62 @@ describe('stringify static html', () => {
216216
expect(ast.hoists).toMatchObject([
217217
{
218218
// the expression and the tree are still hoistable
219-
// but if it's stringified it will be NodeTypes.CALL_EXPRESSION
219+
// but should stay NodeTypes.VNODE_CALL
220+
// if it's stringified it will be NodeTypes.JS_CALL_EXPRESSION
220221
type: NodeTypes.VNODE_CALL
221222
},
222223
{
223224
type: NodeTypes.JS_ARRAY_EXPRESSION
224225
}
225226
])
227+
expect(code).toMatchSnapshot()
228+
})
229+
230+
test('should work with bindings that are non-static but stringifiable', () => {
231+
// if a binding is non-static but marked as CAN_STRINGIFY, it means it's
232+
// a known reference to a constant string.
233+
const { ast, code } = compile(
234+
`<div><div>${repeat(
235+
`<span class="foo">foo</span>`,
236+
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
237+
)}<img src="./foo" /></div></div>`,
238+
{
239+
hoistStatic: true,
240+
prefixIdentifiers: true,
241+
transformHoist: stringifyStatic,
242+
nodeTransforms: [
243+
node => {
244+
if (node.type === NodeTypes.ELEMENT && node.tag === 'img') {
245+
const exp = createSimpleExpression(
246+
'_imports_0_',
247+
false,
248+
node.loc,
249+
ConstantTypes.CAN_STRINGIFY
250+
)
251+
node.props[0] = {
252+
type: NodeTypes.DIRECTIVE,
253+
name: 'bind',
254+
arg: createSimpleExpression('src', true),
255+
exp,
256+
modifiers: [],
257+
loc: node.loc
258+
}
259+
}
260+
}
261+
]
262+
}
263+
)
264+
expect(ast.hoists).toMatchObject([
265+
{
266+
// the hoisted node should be NodeTypes.JS_CALL_EXPRESSION
267+
// of `createStaticVNode()` instead of dynamic NodeTypes.VNODE_CALL
268+
type: NodeTypes.JS_CALL_EXPRESSION
269+
},
270+
{
271+
type: NodeTypes.JS_ARRAY_EXPRESSION
272+
}
273+
])
274+
expect(code).toMatchSnapshot()
226275
})
227276

228277
// #1128

packages/compiler-dom/src/transforms/stringifyStatic.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {
1414
ElementTypes,
1515
PlainElementNode,
1616
JSChildNode,
17-
TextCallNode
17+
TextCallNode,
18+
ConstantTypes
1819
} from '@vue/compiler-core'
1920
import {
2021
isVoidTag,
@@ -171,7 +172,7 @@ const isNonStringifiable = /*#__PURE__*/ makeMap(
171172

172173
/**
173174
* for a hoisted node, analyze it and return:
174-
* - false: bailed (contains runtime constant)
175+
* - false: bailed (contains non-stringifiable props or runtime constant)
175176
* - [nc, ec] where
176177
* - nc is the number of nodes inside
177178
* - ec is the number of element with bindings inside
@@ -216,6 +217,13 @@ function analyzeNode(node: StringifiableNode): [number, number] | false {
216217
) {
217218
return bail()
218219
}
220+
if (
221+
p.exp &&
222+
(p.exp.type === NodeTypes.COMPOUND_EXPRESSION ||
223+
p.exp.constType < ConstantTypes.CAN_STRINGIFY)
224+
) {
225+
return bail()
226+
}
219227
}
220228
}
221229
for (let i = 0; i < node.children.length; i++) {

packages/compiler-sfc/__tests__/templateTransformAssetUrl.spec.ts

-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ describe('compiler sfc: transform asset url', () => {
160160
transformHoist: stringifyStatic
161161
}
162162
)
163-
console.log(code)
164163
expect(code).toMatch(`_createStaticVNode`)
165164
expect(code).toMatchSnapshot()
166165
})

0 commit comments

Comments
 (0)