fix: crashes when v-on expression is not a function (vuejs#2605)

Author: luwuer

packages/compiler-core/__tests__/transforms/__snapshots__/hoistStatic.spec.ts.snap

@@ -209,7 +209,7 @@ export function render(_ctx, _cache) {
   return (_openBlock(), _createBlock(\\"div\\", null, [
     _createVNode(\\"div\\", null, [
       _createVNode(\\"div\\", {
-        onClick: _cache[1] || (_cache[1] = (...args) => (_ctx.foo(...args)))
+        onClick: _cache[1] || (_cache[1] = (...args) => ( (() => { if (typeof _ctx.foo === 'function') { return _ctx.foo } else { console.warn('v-on expression is not a function.') return () => { } } })()(...args)))
       })
     ])
   ]))

packages/compiler-core/__tests__/transforms/vOn.spec.ts

@@ -10,7 +10,7 @@ import {
   transform,
   VNodeCall
 } from '../../src'
-import { transformOn } from '../../src/transforms/vOn'
+import { transformOn, wrapExpressionContent } from '../../src/transforms/vOn'
 import { transformElement } from '../../src/transforms/transformElement'
 import { transformExpression } from '../../src/transforms/transformExpression'
 
@@ -30,6 +30,21 @@ function parseWithVOn(template: string, options: CompilerOptions = {}) {
 }
 
 describe('compiler: transform v-on', () => {
+  test('wrapper', () => {
+    const wrapedExp = wrapExpressionContent('onClick')
+    expect(wrapedExp).toMatch(
+      `
+      (() => {
+          if (typeof onClick === 'function') {
+              return onClick
+          } else {
+              console.warn('v-on expression is not a function.')
+              return () => { }
+          }
+      })()`.replace(/\s+/g, ' ')
+    )
+  })
+
   test('basic', () => {
     const { node } = parseWithVOn(`<div v-on:click="onClick"/>`)
     expect((node.codegenNode as VNodeCall).props).toMatchObject({
@@ -476,7 +491,11 @@ describe('compiler: transform v-on', () => {
         index: 1,
         value: {
           type: NodeTypes.COMPOUND_EXPRESSION,
-          children: [`(...args) => (`, { content: `_ctx.foo(...args)` }, `)`]
+          children: [
+            `(...args) => (`,
+            { content: wrapExpressionContent(`_ctx.foo`) },
+            `)`
+          ]
         }
       })
     })

packages/compiler-core/src/errors.ts

@@ -70,6 +70,7 @@ export const enum ErrorCodes {
   X_V_FOR_TEMPLATE_KEY_PLACEMENT,
   X_V_BIND_NO_EXPRESSION,
   X_V_ON_NO_EXPRESSION,
+  X_V_ON_EXPRESSION_NOT_FUNCTION,
   X_V_SLOT_UNEXPECTED_DIRECTIVE_ON_SLOT_OUTLET,
   X_V_SLOT_MIXED_SLOT_USAGE,
   X_V_SLOT_DUPLICATE_SLOT_NAMES,
@@ -144,6 +145,7 @@ export const errorMessages: { [code: number]: string } = {
   [ErrorCodes.X_V_FOR_TEMPLATE_KEY_PLACEMENT]: `<template v-for> key should be placed on the <template> tag.`,
   [ErrorCodes.X_V_BIND_NO_EXPRESSION]: `v-bind is missing expression.`,
   [ErrorCodes.X_V_ON_NO_EXPRESSION]: `v-on is missing expression.`,
+  [ErrorCodes.X_V_ON_EXPRESSION_NOT_FUNCTION]: `v-on expression is not a function.`,
   [ErrorCodes.X_V_SLOT_UNEXPECTED_DIRECTIVE_ON_SLOT_OUTLET]: `Unexpected custom directive on <slot> outlet.`,
   [ErrorCodes.X_V_SLOT_MIXED_SLOT_USAGE]:
     `Mixed v-slot usage on both the component and nested <template>.` +

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

@@ -1,16 +1,18 @@
 import { DirectiveTransform, DirectiveTransformResult } from '../transform'
 import {
+  CompoundExpressionNode,
   createCompoundExpression,
   createObjectProperty,
   createSimpleExpression,
   DirectiveNode,
   ElementTypes,
   ExpressionNode,
   NodeTypes,
-  SimpleExpressionNode
+  SimpleExpressionNode,
+  SourceLocation
 } from '../ast'
-import { camelize, toHandlerKey } from '@vue/shared'
-import { createCompilerError, ErrorCodes } from '../errors'
+import { camelize, isString, NOOP, toHandlerKey } from '@vue/shared'
+import { createCompilerError, ErrorCodes, errorMessages } from '../errors'
 import { processExpression } from './transformExpression'
 import { validateBrowserExpression } from '../validateExpression'
 import { hasScopeRef, isMemberExpression } from '../utils'
@@ -28,6 +30,51 @@ export interface VOnDirectiveNode extends DirectiveNode {
   exp: SimpleExpressionNode | undefined
 }
 
+export const unfoldExpression = (
+  exp: CompoundExpressionNode
+): SimpleExpressionNode => {
+  let loc: SourceLocation = {
+    source: '',
+    start: { line: 1, column: 1, offset: 0 },
+    end: { line: 1, column: 1, offset: 0 }
+  }
+  let content = ''
+
+  for (let i = 0; i < exp.children!.length; i++) {
+    let node = exp.children[i]
+    if (isString(node)) {
+      loc.source += node
+      content += node
+    } else {
+      let simpleExp = node as SimpleExpressionNode
+      !i && (loc.start = simpleExp.loc.start)
+      loc.end = simpleExp.loc.end
+      loc.source += simpleExp.loc.source
+      content += simpleExp.content
+    }
+  }
+
+  return createSimpleExpression(content, false, loc)
+}
+
+// Event expression wrap to make sure it is a function.
+// See issue #2605.
+export const wrapExpressionContent = (content: string): string => {
+  return (
+    `
+  (() => {
+      if (typeof ${content} === 'function') {
+          return ${content}
+      } else {
+          console.warn('${
+            errorMessages[ErrorCodes.X_V_ON_EXPRESSION_NOT_FUNCTION]
+          }')
+          return ${NOOP}
+      }
+  })()`.replace(/\s+/g, ' ') + `(...args)`
+  )
+}
+
 export const transformOn: DirectiveTransform = (
   dir,
   node,
@@ -102,11 +149,11 @@ export const transformOn: DirectiveTransform = (
       // below) so that it always accesses the latest value when called - thus
       // avoiding the need to be patched.
       if (shouldCache && isMemberExp) {
-        if (exp.type === NodeTypes.SIMPLE_EXPRESSION) {
-          exp.content += `(...args)`
-        } else {
-          exp.children.push(`(...args)`)
+        if (exp.type === NodeTypes.COMPOUND_EXPRESSION) {
+          exp = unfoldExpression(exp)
         }
+
+        exp.content = wrapExpressionContent(exp.content)
       }
     }