Skip to content
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.

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

fix: shouldn't not stringify static for top level inside slot, but ca… #1286

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions packages/compiler-core/src/options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ElementNode, Namespace, TemplateChildNode } from './ast'
import { ElementNode, Namespace, TemplateChildNode, ParentNode } from './ast'
import { TextModes } from './parse'
import { CompilerError } from './errors'
import {
Expand Down Expand Up @@ -53,7 +53,8 @@ export interface ParserOptions {

export type HoistTransform = (
children: TemplateChildNode[],
context: TransformContext
context: TransformContext,
parent: ParentNode
) => void

export interface TransformOptions {
Expand Down
57 changes: 42 additions & 15 deletions packages/compiler-core/src/transforms/hoistStatic.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import {
RootNode,
NodeTypes,
TemplateChildNode,
SimpleExpressionNode,
ComponentNode,
ElementNode,
ElementTypes,
NodeTypes,
ParentNode,
PlainElementNode,
ComponentNode,
RootNode,
SimpleExpressionNode,
TemplateChildNode,
TemplateNode,
ElementNode,
VNodeCall
} from '../ast'
import { TransformContext } from '../transform'
import { PatchFlags, isString, isSymbol } from '@vue/shared'
import { isSlotOutlet, findProp } from '../utils'
import { isString, isSymbol, PatchFlags } from '@vue/shared'
import { findProp, isSlotOutlet } from '../utils'

export function hoistStatic(root: RootNode, context: TransformContext) {
walk(
root.children,
root,
context,
new Map(),
// Root node is unfortunately non-hoistable due to potential parent
Expand Down Expand Up @@ -44,7 +45,7 @@ const enum StaticType {
}

function walk(
children: TemplateChildNode[],
parent: ParentNode,
context: TransformContext,
resultCache: Map<TemplateChildNode, StaticType>,
doNotHoistNode: boolean = false
Expand All @@ -59,7 +60,7 @@ function walk(
// walk of the AST and allow `stringifyStatic` to stop walking as soon as its
// stringficiation threshold is met.
let hasRuntimeConstant = false

const children = parent.children
for (let i = 0; i < children.length; i++) {
const child = children[i]
// only plain elements & text calls are eligible for hoisting.
Expand Down Expand Up @@ -114,21 +115,47 @@ function walk(

// walk further
if (child.type === NodeTypes.ELEMENT) {
walk(child.children, context, resultCache)
walk(child, context, resultCache)
} else if (child.type === NodeTypes.FOR) {
// Do not hoist v-for single child because it has to be a block
walk(child.children, context, resultCache, child.children.length === 1)
walk(child, context, resultCache, child.children.length === 1)
} else if (child.type === NodeTypes.IF) {
for (let i = 0; i < child.branches.length; i++) {
const branchChildren = child.branches[i].children
// Do not hoist v-if single child because it has to be a block
walk(branchChildren, context, resultCache, branchChildren.length === 1)
walk(
child.branches[i],
context,
resultCache,
branchChildren.length === 1
)
}
}
}

if (!hasRuntimeConstant && hasHoistedNode && context.transformHoist) {
context.transformHoist(children, context)
if (
parent.type === NodeTypes.ELEMENT &&
(parent.tagType === ElementTypes.COMPONENT ||
parent.tagType === ElementTypes.TEMPLATE)
) {
// slot case
// shouldn't stringify root level node inside slot, but the children of themselves can be stringify
children.forEach(child => {
if (
child.type === NodeTypes.ELEMENT ||
child.type === NodeTypes.COMPOUND_EXPRESSION
) {
context.transformHoist!(
child.children as TemplateChildNode[],
context,
child as ElementNode
)
}
})
} else {
context.transformHoist(children, context, parent)
}
}
}

Expand Down
80 changes: 74 additions & 6 deletions packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import {
compile,
NodeTypes,
CREATE_STATIC,
createSimpleExpression
createSimpleExpression,
generate
} from '../../src'
import {
stringifyStatic,
Expand Down Expand Up @@ -89,11 +90,8 @@ describe('stringify static html', () => {
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}</div>`
)
// should have 5 hoisted nodes, but the other 4 should be null
expect(ast.hoists.length).toBe(5)
for (let i = 1; i < 5; i++) {
expect(ast.hoists[i]).toBe(null)
}

expect(ast.hoists.length).toBe(1)
// should be optimized now
expect(ast.hoists[0]).toMatchObject({
type: NodeTypes.JS_CALL_EXPRESSION,
Expand Down Expand Up @@ -225,4 +223,74 @@ describe('stringify static html', () => {
type: NodeTypes.VNODE_CALL // not CALL_EXPRESSION
})
})

test('should not work on root level element inside slot', () => {
const { ast } = compileWithStringify(
`<comp>${repeat(
`<span class="foo"/>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}</comp>`
)
expect(generate(ast).code).toMatchInlineSnapshot(`
"const _Vue = Vue
const { createVNode: _createVNode } = _Vue

const _hoisted_1 = /*#__PURE__*/_createVNode(\\"span\\", { class: \\"foo\\" }, [], -1 /* HOISTED */)
const _hoisted_2 = /*#__PURE__*/_createVNode(\\"span\\", { class: \\"foo\\" }, [], -1 /* HOISTED */)
const _hoisted_3 = /*#__PURE__*/_createVNode(\\"span\\", { class: \\"foo\\" }, [], -1 /* HOISTED */)
const _hoisted_4 = /*#__PURE__*/_createVNode(\\"span\\", { class: \\"foo\\" }, [], -1 /* HOISTED */)
const _hoisted_5 = /*#__PURE__*/_createVNode(\\"span\\", { class: \\"foo\\" }, [], -1 /* HOISTED */)

return function render(_ctx, _cache) {
with (_ctx) {
const { createVNode: _createVNode, resolveComponent: _resolveComponent, withCtx: _withCtx, openBlock: _openBlock, createBlock: _createBlock } = _Vue

const _component_comp = _resolveComponent(\\"comp\\")

return (_openBlock(), _createBlock(_component_comp, null, {
default: _withCtx(() => [
_hoisted_1,
_hoisted_2,
_hoisted_3,
_hoisted_4,
_hoisted_5
]),
_: 1
}))
}
}"
`)
})

test('should work on inner elements inside top level elements in slot', () => {
const { ast } = compileWithStringify(
`<comp><div>${repeat(
`<span class="foo"/>`,
StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
)}</div></comp>`
)
expect(generate(ast).code).toMatchInlineSnapshot(`
"const _Vue = Vue
const { createVNode: _createVNode, createStaticVNode: _createStaticVNode } = _Vue

const _hoisted_1 = /*#__PURE__*/_createVNode(\\"div\\", null, [
/*#__PURE__*/_createStaticVNode(\\"<span class=\\\\\\"foo\\\\\\"></span><span class=\\\\\\"foo\\\\\\"></span><span class=\\\\\\"foo\\\\\\"></span><span class=\\\\\\"foo\\\\\\"></span><span class=\\\\\\"foo\\\\\\"></span>\\", 5)
], -1 /* HOISTED */)

return function render(_ctx, _cache) {
with (_ctx) {
const { createVNode: _createVNode, resolveComponent: _resolveComponent, withCtx: _withCtx, createStaticVNode: _createStaticVNode, openBlock: _openBlock, createBlock: _createBlock } = _Vue

const _component_comp = _resolveComponent(\\"comp\\")

return (_openBlock(), _createBlock(_component_comp, null, {
default: _withCtx(() => [
_hoisted_1
]),
_: 1
}))
}
}"
`)
})
})
100 changes: 53 additions & 47 deletions packages/compiler-dom/src/transforms/stringifyStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@
* This module is Node-only.
*/
import {
NodeTypes,
ElementNode,
TransformContext,
TemplateChildNode,
SimpleExpressionNode,
createCallExpression,
HoistTransform,
CREATE_STATIC,
ExpressionNode,
createCallExpression,
ElementNode,
ElementTypes,
PlainElementNode,
ExpressionNode,
HoistTransform,
JSChildNode,
TextCallNode
NodeTypes,
PlainElementNode,
SimpleExpressionNode,
TemplateChildNode,
TextCallNode,
TransformContext
} from '@vue/compiler-core'
import {
isVoidTag,
escapeHtml,
isKnownAttr,
isString,
isSymbol,
isKnownAttr,
escapeHtml,
toDisplayString,
isVoidTag,
normalizeClass,
normalizeStyle,
stringifyStyle
stringifyStyle,
toDisplayString
} from '@vue/shared'

export const enum StringifyThresholds {
Expand Down Expand Up @@ -58,12 +58,14 @@ type StringifiableNode = PlainElementNode | TextCallNode
*
* This optimization is only performed in Node.js.
*/
export const stringifyStatic: HoistTransform = (children, context) => {
export const stringifyStatic: HoistTransform = (children, context, parent) => {
let nc = 0 // current node count
let ec = 0 // current element with binding count
const currentChunk: StringifiableNode[] = []
const chunks: (JSChildNode | TemplateChildNode)[] = []
const parentIsHoistedNode = getHoistedNode(parent as TemplateChildNode)

const stringifyCurrentChunk = (currentIndex: number): number => {
const pushChunk = () => {
if (
nc >= StringifyThresholds.NODE_COUNT ||
ec >= StringifyThresholds.ELEMENT_WITH_BINDING_COUNT
Expand All @@ -77,51 +79,56 @@ export const stringifyStatic: HoistTransform = (children, context) => {
// will insert / hydrate
String(currentChunk.length)
])
// replace the first node's hoisted expression with the static vnode call
replaceHoist(currentChunk[0], staticCall, context)

if (currentChunk.length > 1) {
for (let i = 1; i < currentChunk.length; i++) {
// for the merged nodes, set their hoisted expression to null
replaceHoist(currentChunk[i], null, context)
if (parentIsHoistedNode) {
// if parent is hoisted, just append staticCall instead of hoisted vnode list
chunks.push(staticCall)
} else {
// if parent isn't hoisted, this mean is hoisted node self.
// so need remove chunk nodes, re-create hoist for staticCall
for (let i = 0; i < currentChunk.length; i++) {
removeHoist(currentChunk[i], context)
}

// also remove merged nodes from children
const deleteCount = currentChunk.length - 1
children.splice(currentIndex - currentChunk.length + 1, deleteCount)
return deleteCount
chunks.push(context.hoist(staticCall))
}
currentChunk.length = 0
}
// if the nodes of currentChunk can't static, should append to chunks
if (currentChunk.length) {
chunks.push(...currentChunk)
}
return 0
}

let i = 0
for (; i < children.length; i++) {
const child = children[i]
const hoisted = getHoistedNode(child)
if (hoisted) {
if (getHoistedNode(child) || parentIsHoistedNode) {
// presence of hoisted means child must be a stringifiable node
const node = child as StringifiableNode
const result = analyzeNode(node)
const result = analyzeNode(child)
if (result) {
// node is stringifiable, record state
nc += result[0]
ec += result[1]
currentChunk.push(node)
currentChunk.push(child as StringifiableNode)
continue
}
}
// we only reach here if we ran into a node that is not stringifiable
// check if currently analyzed nodes meet criteria for stringification.
// adjust iteration index
i -= stringifyCurrentChunk(i)
pushChunk()
// current node should append to chunks
chunks.push(child)
// reset state
nc = 0
ec = 0
currentChunk.length = 0
}
// in case the last node was also stringifiable
stringifyCurrentChunk(i)
// maybe current chunk has children
pushChunk()
if (parentIsHoistedNode) {
(parent as any).codegenNode.hoisted!.children = chunks as TemplateChildNode[]
} else {
parent.children = chunks as TemplateChildNode[]
}
}

const getHoistedNode = (node: TemplateChildNode) =>
Expand All @@ -136,13 +143,9 @@ const isStringifiableAttr = (name: string) => {
return isKnownAttr(name) || dataAriaRE.test(name)
}

const replaceHoist = (
node: StringifiableNode,
replacement: JSChildNode | null,
context: TransformContext
) => {
const hoistToReplace = (node.codegenNode as SimpleExpressionNode).hoisted!
context.hoists[context.hoists.indexOf(hoistToReplace)] = replacement
const removeHoist = (node: StringifiableNode, context: TransformContext) => {
const hoistToRemove = (node.codegenNode as SimpleExpressionNode).hoisted!
context.hoists.splice(context.hoists.indexOf(hoistToRemove), 1)
}

/**
Expand All @@ -152,10 +155,13 @@ const replaceHoist = (
* - nc is the number of nodes inside
* - ec is the number of element with bindings inside
*/
function analyzeNode(node: StringifiableNode): [number, number] | false {
function analyzeNode(node: TemplateChildNode): [number, number] | false {
if (node.type === NodeTypes.TEXT_CALL) {
return [1, 0]
}
if (node.type !== NodeTypes.ELEMENT) {
return false
}

let nc = 1 // node count
let ec = node.props.length > 0 ? 1 : 0 // element w/ binding count
Expand Down