Skip to content

Commit

Permalink
fix(compiler-core): warn for empty vOn expression + improve performan…
Browse files Browse the repository at this point in the history
…ce of detect cache props

fix vuejs#1716
  • Loading branch information
underfin committed Jul 27, 2020
1 parent 3d2bdaf commit 5898508
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 18 deletions.
31 changes: 23 additions & 8 deletions packages/compiler-core/src/transforms/hoistStatic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function walk(
let staticType
if (
!doNotHoistNode &&
(staticType = getStaticType(child, resultCache)) > 0
(staticType = getStaticType(context, child, resultCache)) > 0
) {
if (staticType === StaticType.HAS_RUNTIME_CONSTANT) {
hasRuntimeConstant = true
Expand All @@ -94,7 +94,7 @@ function walk(
flag === PatchFlags.NEED_PATCH ||
flag === PatchFlags.TEXT) &&
!hasDynamicKeyOrRef(child) &&
!hasCachedProps(child)
!hasCachedProps(child, context)
) {
const props = getNodeProps(child)
if (props) {
Expand All @@ -104,7 +104,7 @@ function walk(
}
}
} else if (child.type === NodeTypes.TEXT_CALL) {
const staticType = getStaticType(child.content, resultCache)
const staticType = getStaticType(context, child.content, resultCache)
if (staticType > 0) {
if (staticType === StaticType.HAS_RUNTIME_CONSTANT) {
hasRuntimeConstant = true
Expand Down Expand Up @@ -139,6 +139,7 @@ function walk(
}

export function getStaticType(
context: TransformContext,
node: TemplateChildNode | SimpleExpressionNode,
resultCache: Map<TemplateChildNode, StaticType> = new Map()
): StaticType {
Expand All @@ -156,11 +157,19 @@ export function getStaticType(
return StaticType.NOT_STATIC
}
const flag = getPatchFlag(codegenNode)
if (!flag && !hasDynamicKeyOrRef(node) && !hasCachedProps(node)) {
if (
!flag &&
!hasDynamicKeyOrRef(node) &&
!hasCachedProps(node, context)
) {
// element self is static. check its children.
let returnType = StaticType.FULL_STATIC
for (let i = 0; i < node.children.length; i++) {
const childType = getStaticType(node.children[i], resultCache)
const childType = getStaticType(
context,
node.children[i],
resultCache
)
if (childType === StaticType.NOT_STATIC) {
resultCache.set(node, StaticType.NOT_STATIC)
return StaticType.NOT_STATIC
Expand Down Expand Up @@ -207,7 +216,7 @@ export function getStaticType(
return StaticType.NOT_STATIC
case NodeTypes.INTERPOLATION:
case NodeTypes.TEXT_CALL:
return getStaticType(node.content, resultCache)
return getStaticType(context, node.content, resultCache)
case NodeTypes.SIMPLE_EXPRESSION:
return node.isConstant
? node.isRuntimeConstant
Expand All @@ -221,7 +230,7 @@ export function getStaticType(
if (isString(child) || isSymbol(child)) {
continue
}
const childType = getStaticType(child, resultCache)
const childType = getStaticType(context, child, resultCache)
if (childType === StaticType.NOT_STATIC) {
return StaticType.NOT_STATIC
} else if (childType === StaticType.HAS_RUNTIME_CONSTANT) {
Expand All @@ -242,10 +251,16 @@ function hasDynamicKeyOrRef(node: ElementNode): boolean {
return !!(findProp(node, 'key', true) || findProp(node, 'ref', true))
}

function hasCachedProps(node: PlainElementNode): boolean {
function hasCachedProps(
node: PlainElementNode,
context: TransformContext
): boolean {
if (__BROWSER__) {
return false
}
if (!context.cacheHandlers) {
return false
}
const props = getNodeProps(node)
if (props && props.type === NodeTypes.JS_OBJECT_EXPRESSION) {
const { properties } = props
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-core/src/transforms/transformElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export const transformElement: NodeTransform = (node, context) => {
const hasDynamicTextChild =
type === NodeTypes.INTERPOLATION ||
type === NodeTypes.COMPOUND_EXPRESSION
if (hasDynamicTextChild && !getStaticType(child)) {
if (hasDynamicTextChild && !getStaticType(context, child)) {
patchFlag |= PatchFlags.TEXT
}
// pass directly if the only child is a text node
Expand Down Expand Up @@ -293,7 +293,7 @@ export function buildProps(
value.type === NodeTypes.JS_CACHE_EXPRESSION ||
((value.type === NodeTypes.SIMPLE_EXPRESSION ||
value.type === NodeTypes.COMPOUND_EXPRESSION) &&
getStaticType(value) > 0)
getStaticType(context, value) > 0)
) {
// skip if the prop is a cached handler or has constant value
return
Expand Down
16 changes: 8 additions & 8 deletions packages/compiler-core/src/transforms/vOn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ export const transformOn: DirectiveTransform = (
augmentor
) => {
const { loc, modifiers, arg } = dir as VOnDirectiveNode
if (!dir.exp && !modifiers.length) {
let exp: ExpressionNode | undefined = dir.exp as
| SimpleExpressionNode
| undefined
if (exp && !exp.content.trim()) {
exp = undefined
}
if (!exp && !modifiers.length) {
context.onError(createCompilerError(ErrorCodes.X_V_ON_NO_EXPRESSION, loc))
}
let eventName: ExpressionNode
Expand All @@ -62,13 +68,7 @@ export const transformOn: DirectiveTransform = (
}

// handler processing
let exp: ExpressionNode | undefined = dir.exp as
| SimpleExpressionNode
| undefined
if (exp && !exp.content.trim()) {
exp = undefined
}
let isCacheable: boolean = !exp
let isCacheable: boolean = __BROWSER__ ? false : context.cacheHandlers && !exp
if (exp) {
const isMemberExp = isMemberExpression(exp.content)
const isInlineStatement = !(isMemberExp || fnExpRE.test(exp.content))
Expand Down

0 comments on commit 5898508

Please sign in to comment.