Repository: vuejs/vue
Pull request: fix(compiler): return handler value for event modifiers instead of un…

fix(compiler): return handler value for event modifiers instead of undefined

<!--
Please make sure to read the Pull Request Guidelines:
https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#pull-request-guidelines
-->

<!-- PULL REQUEST TEMPLATE -->
<!-- (Update "[ ]" to "[x]" to check a box) -->

**What kind of change does this PR introduce?** (check at least one)

- [x] Bugfix
- [ ] Feature
- [ ] Code style update
- [ ] Refactor
- [ ] Build-related changes
- [ ] Other, please describe:

**Does this PR introduce a breaking change?** (check one)

- [ ] Yes
- [x] No

If yes, please describe the impact and migration path for existing applications:

**The PR fulfills these requirements:**

- [x] It's submitted to the `dev` branch for v2.x (or to a previous version branch), _not_ the `master` branch
- [x] When resolving a specific issue, it's referenced in the PR's title (e.g. `fix #xxx[,#xxx]`, where "xxx" is the issue number)
- [x] All tests are passing: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#development-setup
- [x] New/updated tests are included

If adding a **new feature**, the PR's description includes:
- [x] A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

**Other information:**
Without modifiers compiler return original vm function, so we can expect to handle returned value (as example - handle it in directive)
```
on: {
    click:  _vm.test($event)
}
```

With modifier it always return "undefined" because of missing return value
```
on: {
    click: function($event) {
        $event.preventDefault()
        _vm.test($event)
    }
}
```

This PR add correct return value if modifier wrapper is used
```
on: {
    click: function($event) {
        $event.preventDefault()
        return _vm.test($event)
    }
}
```

Changed files:
filename: src/compiler/codegen/events.js
patch: @@ -132,9 +132,9 @@ function genHandler (
       code += genModifierCode
     }
     const handlerCode = isMethodPath
-      ? handler.value + '($event)'
+      ? `return ${handler.value}($event)`
       : isFunctionExpression
-        ? `(${handler.value})($event)`
+        ? `return (${handler.value})($event)`
         : handler.value
     /* istanbul ignore if */
     if (__WEEX__ && handler.params) {

filename: test/unit/modules/compiler/codegen.spec.js
patch: @@ -269,120 +269,165 @@ describe('codegen', () => {
     )
   })
 
+  it('generate events with method call', () => {
+    assertCodegen(
+      '<input @input="onInput($event);">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput($event);}}})}`
+    )
+    // empty arguments
+    assertCodegen(
+      '<input @input="onInput();">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput();}}})}`
+    )
+    // without semicolon
+    assertCodegen(
+      '<input @input="onInput($event)">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput($event)}}})}`
+    )
+    // multiple args
+    assertCodegen(
+      '<input @input="onInput($event, \'abc\', 5);">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput($event, 'abc', 5);}}})}`
+    )
+    // expression in args
+    assertCodegen(
+      '<input @input="onInput($event, 2+2);">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput($event, 2+2);}}})}`
+    )
+    // tricky symbols in args
+    assertCodegen(
+      '<input @input="onInput(\');[\'());\');">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput(');[\'());');}}})}`
+    )
+  })
+
+  it('generate events with multiple statements', () => {
+    // normal function
+    assertCodegen(
+      '<input @input="onInput1();onInput2()">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput1();onInput2()}}})}`
+    )
+    // function with multiple args
+    assertCodegen(
+      '<input @input="onInput1($event, \'text\');onInput2(\'text2\', $event)">',
+      `with(this){return _c('input',{on:{"input":function($event){onInput1($event, 'text');onInput2('text2', $event)}}})}`
+    )
+  })
+
   it('generate events with keycode', () => {
     assertCodegen(
       '<input @input.enter="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;return onInput($event)}}})}`
     )
     // multiple keycodes (delete)
     assertCodegen(
       '<input @input.delete="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"delete",[8,46],$event.key,["Backspace","Delete"]))return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"delete",[8,46],$event.key,["Backspace","Delete"]))return null;return onInput($event)}}})}`
     )
     // multiple keycodes (chained)
     assertCodegen(
       '<input @keydown.enter.delete="onInput">',
-      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter")&&_k($event.keyCode,"delete",[8,46],$event.key,["Backspace","Delete"]))return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter")&&_k($event.keyCode,"delete",[8,46],$event.key,["Backspace","Delete"]))return null;return onInput($event)}}})}`
     )
     // number keycode
     assertCodegen(
       '<input @input.13="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&$event.keyCode!==13)return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&$event.keyCode!==13)return null;return onInput($event)}}})}`
     )
     // custom keycode
     assertCodegen(
       '<input @input.custom="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"custom",undefined,$event.key,undefined))return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){if(!('button' in $event)&&_k($event.keyCode,"custom",undefined,$event.key,undefined))return null;return onInput($event)}}})}`
     )
   })
 
   it('generate events with generic modifiers', () => {
     assertCodegen(
       '<input @input.stop="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){$event.stopPropagation();onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){$event.stopPropagation();return onInput($event)}}})}`
     )
     assertCodegen(
       '<input @input.prevent="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){$event.preventDefault();onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){$event.preventDefault();return onInput($event)}}})}`
     )
     assertCodegen(
       '<input @input.self="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){if($event.target !== $event.currentTarget)return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){if($event.target !== $event.currentTarget)return null;return onInput($event)}}})}`
     )
   })
 
   // GitHub Issues #5146
   it('generate events with generic modifiers and keycode correct order', () => {
     assertCodegen(
       '<input @keydown.enter.prevent="onInput">',
-      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;$event.preventDefault();onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;$event.preventDefault();return onInput($event)}}})}`
     )
 
     assertCodegen(
       '<input @keydown.enter.stop="onInput">',
-      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;$event.stopPropagation();onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"keydown":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;$event.stopPropagation();return onInput($event)}}})}`
     )
   })
 
   it('generate events with mouse event modifiers', () => {
     assertCodegen(
       '<input @click.ctrl="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if(!$event.ctrlKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if(!$event.ctrlKey)return null;return onClick($event)}}})}`
     )
     assertCodegen(
       '<input @click.shift="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if(!$event.shiftKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if(!$event.shiftKey)return null;return onClick($event)}}})}`
     )
     assertCodegen(
       '<input @click.alt="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if(!$event.altKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if(!$event.altKey)return null;return onClick($event)}}})}`
     )
     assertCodegen(
       '<input @click.meta="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if(!$event.metaKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if(!$event.metaKey)return null;return onClick($event)}}})}`
     )
     assertCodegen(
       '<input @click.exact="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if($event.ctrlKey||$event.shiftKey||$event.altKey||$event.metaKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if($event.ctrlKey||$event.shiftKey||$event.altKey||$event.metaKey)return null;return onClick($event)}}})}`
     )
     assertCodegen(
       '<input @click.ctrl.exact="onClick">',
-      `with(this){return _c('input',{on:{"click":function($event){if(!$event.ctrlKey)return null;if($event.shiftKey||$event.altKey||$event.metaKey)return null;onClick($event)}}})}`
+      `with(this){return _c('input',{on:{"click":function($event){if(!$event.ctrlKey)return null;if($event.shiftKey||$event.altKey||$event.metaKey)return null;return onClick($event)}}})}`
     )
   })
 
   it('generate events with multiple modifiers', () => {
     assertCodegen(
       '<input @input.stop.prevent.self="onInput">',
-      `with(this){return _c('input',{on:{"input":function($event){$event.stopPropagation();$event.preventDefault();if($event.target !== $event.currentTarget)return null;onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"input":function($event){$event.stopPropagation();$event.preventDefault();if($event.target !== $event.currentTarget)return null;return onInput($event)}}})}`
     )
   })
 
   it('generate events with capture modifier', () => {
     assertCodegen(
       '<input @input.capture="onInput">',
-      `with(this){return _c('input',{on:{"!input":function($event){onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"!input":function($event){return onInput($event)}}})}`
     )
   })
 
   it('generate events with once modifier', () => {
     assertCodegen(
       '<input @input.once="onInput">',
-      `with(this){return _c('input',{on:{"~input":function($event){onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"~input":function($event){return onInput($event)}}})}`
     )
   })
 
   it('generate events with capture and once modifier', () => {
     assertCodegen(
       '<input @input.capture.once="onInput">',
-      `with(this){return _c('input',{on:{"~!input":function($event){onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"~!input":function($event){return onInput($event)}}})}`
     )
   })
 
   it('generate events with once and capture modifier', () => {
     assertCodegen(
       '<input @input.once.capture="onInput">',
-      `with(this){return _c('input',{on:{"~!input":function($event){onInput($event)}}})}`
+      `with(this){return _c('input',{on:{"~!input":function($event){return onInput($event)}}})}`
     )
   })
 
@@ -427,7 +472,7 @@ describe('codegen', () => {
     // with modifiers
     assertCodegen(
       `<input @keyup.enter="e=>current++">`,
-      `with(this){return _c('input',{on:{"keyup":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;(e=>current++)($event)}}})}`
+      `with(this){return _c('input',{on:{"keyup":function($event){if(!('button' in $event)&&_k($event.keyCode,"enter",13,$event.key,"Enter"))return null;return (e=>current++)($event)}}})}`
     )
   })
 
@@ -452,7 +497,7 @@ describe('codegen', () => {
   it('generate multiple event handlers', () => {
     assertCodegen(
       '<input @input="current++" @input.stop="onInput">',
-      `with(this){return _c('input',{on:{"input":[function($event){current++},function($event){$event.stopPropagation();onInput($event)}]}})}`
+      `with(this){return _c('input',{on:{"input":[function($event){current++},function($event){$event.stopPropagation();return onInput($event)}]}})}`
     )
   })
 

filename: test/unit/modules/vdom/patch/edge-cases.spec.js
patch: @@ -25,6 +25,43 @@ describe('vdom patch: edge cases', () => {
     }).then(done)
   })
 
+  // exposed by #7705
+  // methods and function expressions with modifiers should return result instead of undefined
+  // skipped odd children[1,3, ...] because they are rendered as text nodes with undefined value
+  it('should return listener\'s result for method name and function expression with and w/o modifiers', done => {
+    const dummyEvt = { preventDefault: () => {} }
+    new Vue({
+      template: `
+        <div v-test>
+          <div @click="addFive"></div>
+          <div @click.prevent="addFive"></div>
+          <div @click="addFive($event, 5)"></div>
+          <div @click.prevent="addFive($event, 5)"></div>
+        </div>
+      `,
+      methods: {
+        addFive ($event, toAdd = 0) {
+          return toAdd + 5
+        }
+      },
+      directives: {
+        test: {
+          bind (el, binding, vnode) {
+            waitForUpdate(() => {
+              expect(vnode.children[0].data.on.click()).toBe(5)
+            }).then(() => {
+              expect(vnode.children[2].data.on.click(dummyEvt)).toBe(5)
+            }).then(() => {
+              expect(vnode.children[4].data.on.click()).not.toBeDefined()
+            }).then(() => {
+              expect(vnode.children[6].data.on.click(dummyEvt)).not.toBeDefined()
+            }).then(done)
+          }
+        }
+      }
+    }).$mount()
+  })
+
   // #3533
   // a static node is reused in createElm, which changes its elm reference
   // and is inserted into a different parent.