Skip to content

Commit

Permalink
fix(compiler-dom): avoid bailing stringification on setup const bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Dec 6, 2021
1 parent 4713578 commit 29beda7
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 31 deletions.
32 changes: 7 additions & 25 deletions packages/compiler-core/src/transforms/hoistStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,6 @@ function walk(
context: TransformContext,
doNotHoistNode: boolean = false
) {
// Some transforms, e.g. transformAssetUrls from @vue/compiler-sfc, replaces
// static bindings with expressions. These expressions are guaranteed to be
// constant so they are still eligible for hoisting, but they are only
// available at runtime and therefore cannot be evaluated ahead of time.
// This is only a concern for pre-stringification (via transformHoist by
// @vue/compiler-dom), but doing it here allows us to perform only one full
// walk of the AST and allow `stringifyStatic` to stop walking as soon as its
// stringification threshold is met.
let canStringify = true

const { children } = node
const originalCount = children.length
let hoistedCount = 0
Expand All @@ -77,9 +67,6 @@ function walk(
? ConstantTypes.NOT_CONSTANT
: getConstantType(child, context)
if (constantType > ConstantTypes.NOT_CONSTANT) {
if (constantType < ConstantTypes.CAN_STRINGIFY) {
canStringify = false
}
if (constantType >= ConstantTypes.CAN_HOIST) {
;(child.codegenNode as VNodeCall).patchFlag =
PatchFlags.HOISTED + (__DEV__ ? ` /* HOISTED */` : ``)
Expand Down Expand Up @@ -110,17 +97,12 @@ function walk(
}
}
}
} else if (child.type === NodeTypes.TEXT_CALL) {
const contentType = getConstantType(child.content, context)
if (contentType > 0) {
if (contentType < ConstantTypes.CAN_STRINGIFY) {
canStringify = false
}
if (contentType >= ConstantTypes.CAN_HOIST) {
child.codegenNode = context.hoist(child.codegenNode)
hoistedCount++
}
}
} else if (
child.type === NodeTypes.TEXT_CALL &&
getConstantType(child.content, context) >= ConstantTypes.CAN_HOIST
) {
child.codegenNode = context.hoist(child.codegenNode)
hoistedCount++
}

// walk further
Expand Down Expand Up @@ -148,7 +130,7 @@ function walk(
}
}

if (canStringify && hoistedCount && context.transformHoist) {
if (hoistedCount && context.transformHoist) {
context.transformHoist(children, context, node)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`stringify static html should bail on bindings that are hoisted but not stringifiable 1`] = `
"const { createElementVNode: _createElementVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
const _hoisted_1 = /*#__PURE__*/_createElementVNode(\\"div\\", null, [
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
/*#__PURE__*/_createElementVNode(\\"span\\", { class: \\"foo\\" }, \\"foo\\"),
/*#__PURE__*/_createElementVNode(\\"img\\", { src: _imports_0_ })
], -1 /* HOISTED */)
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
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)
const _hoisted_2 = [
_hoisted_1
]
return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock(\\"div\\", null, _hoisted_2))
}"
`;
55 changes: 52 additions & 3 deletions packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ describe('stringify static html', () => {
])
})

test('should bail on runtime constant v-bind bindings', () => {
const { ast } = compile(
test('should bail on bindings that are hoisted but not stringifiable', () => {
const { ast, code } = compile(
`<div><div>${repeat(
`<span class="foo">foo</span>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
Expand Down Expand Up @@ -216,13 +216,62 @@ describe('stringify static html', () => {
expect(ast.hoists).toMatchObject([
{
// the expression and the tree are still hoistable
// but if it's stringified it will be NodeTypes.CALL_EXPRESSION
// but should stay NodeTypes.VNODE_CALL
// if it's stringified it will be NodeTypes.JS_CALL_EXPRESSION
type: NodeTypes.VNODE_CALL
},
{
type: NodeTypes.JS_ARRAY_EXPRESSION
}
])
expect(code).toMatchSnapshot()
})

test('should work with bindings that are non-static but stringifiable', () => {
// if a binding is non-static but marked as CAN_STRINGIFY, it means it's
// a known reference to a constant string.
const { ast, code } = compile(
`<div><div>${repeat(
`<span class="foo">foo</span>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}<img src="./foo" /></div></div>`,
{
hoistStatic: true,
prefixIdentifiers: true,
transformHoist: stringifyStatic,
nodeTransforms: [
node => {
if (node.type === NodeTypes.ELEMENT && node.tag === 'img') {
const exp = createSimpleExpression(
'_imports_0_',
false,
node.loc,
ConstantTypes.CAN_STRINGIFY
)
node.props[0] = {
type: NodeTypes.DIRECTIVE,
name: 'bind',
arg: createSimpleExpression('src', true),
exp,
modifiers: [],
loc: node.loc
}
}
}
]
}
)
expect(ast.hoists).toMatchObject([
{
// the hoisted node should be NodeTypes.JS_CALL_EXPRESSION
// of `createStaticVNode()` instead of dynamic NodeTypes.VNODE_CALL
type: NodeTypes.JS_CALL_EXPRESSION
},
{
type: NodeTypes.JS_ARRAY_EXPRESSION
}
])
expect(code).toMatchSnapshot()
})

// #1128
Expand Down
12 changes: 10 additions & 2 deletions packages/compiler-dom/src/transforms/stringifyStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
ElementTypes,
PlainElementNode,
JSChildNode,
TextCallNode
TextCallNode,
ConstantTypes
} from '@vue/compiler-core'
import {
isVoidTag,
Expand Down Expand Up @@ -171,7 +172,7 @@ const isNonStringifiable = /*#__PURE__*/ makeMap(

/**
* for a hoisted node, analyze it and return:
* - false: bailed (contains runtime constant)
* - false: bailed (contains non-stringifiable props or runtime constant)
* - [nc, ec] where
* - nc is the number of nodes inside
* - ec is the number of element with bindings inside
Expand Down Expand Up @@ -216,6 +217,13 @@ function analyzeNode(node: StringifiableNode): [number, number] | false {
) {
return bail()
}
if (
p.exp &&
(p.exp.type === NodeTypes.COMPOUND_EXPRESSION ||
p.exp.constType < ConstantTypes.CAN_STRINGIFY)
) {
return bail()
}
}
}
for (let i = 0; i < node.children.length; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ describe('compiler sfc: transform asset url', () => {
transformHoist: stringifyStatic
}
)
console.log(code)
expect(code).toMatch(`_createStaticVNode`)
expect(code).toMatchSnapshot()
})
Expand Down

0 comments on commit 29beda7

Please sign in to comment.