fix(compiler): return handler value for event modifiers instead of un…

[STUkh]

fix(compiler): return handler value for event modifiers instead of undefined

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch for v2.x (or to a previous version branch), not the master branch
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#development-setup
• New/updated tests are included

If adding a new feature, the PR's description includes:

• 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)
    }
}

[jacekkarczmarczyk]

How will it behave with the following example? https://codepen.io/anon/pen/ddwWKW

The handler code is alert('x'); alert('y'), so it will be compiled to return alert('x'); alert('y') and the second alert won't be shown, am I right?

[Kingwl]

maybe handlerCode should be wrap in a IIFE

[STUkh]

Agree with @jacekkarczmarczyk.
@posva , @Kingwl : Now return added only for single handler expressions. And result all the same with and without modifiers
@click="test" =>click: _vm.test
@click="test($event)" => click: function($event) { return _vm.test($event) }
@click="test($event)**;**" => click: function($event) { return _vm.test($event) }
@click="test1($event);test2($event)" =>

click: function($event) {  
    _vm.test1($event)
    _vm.test2($event)
}

And the same behavior with modifiers

[jacekkarczmarczyk]

What about @click="alert(';')"?
split(';') doesnt' seem to be the good solution

[posva]

Maybe we should return only when the provided expression is a function: <div @click="doSomething"> but not when it is a statement or expression: <div @click="doSomething()"> It always possible to wrap things when you need to pass parameters:

<button @click=" e => doSomething('a parameter', e)">Click me</button>

[STUkh]

@posva you're totally right. But there is a problem when we use modifiers. They wraps handler and we can't get result.

For now i'm stuck at parsing handler to check is multiple expressions inside of it
/\)(?:\s*);(?:\s*)(?!.*'|")/
http://take.ms/TcERo
If it contains single expression - add return; If not - leave handler as is.
A will be appreciate if someone help to check it correctly.
@jacekkarczmarczyk, @posva, @Kingwl maybe you could help? :)

[Kingwl]

thanks for your pr
please add more tests for the case

For now i'm stuck at parsing handler to check is multiple expressions inside of it
/)(?:\s*);(?:\s*)(?!.*'|")/

IMO parse expression with regex or other string operation is not a good chooice
we need to reconsider this feature
ping @yyx990803

[jacekkarczmarczyk]

But there is a problem when we use modifiers. They wraps handler and we can't get result.

I didn't dig deep into the code so I'm not sure what exactly you mean by it, but maybe there's an easy way to implement something like handler.rawValue to get access to non-wrapped handler?

[Kingwl]

implement something like handler.rawValue to get access to non-wrapped handler?

we have not instanced the rawValue yet, the modifiers generated code has mixin with handler.
instanced the handler.rawValue will lead to runtime code size increases

Maybe we can implement it in a limited way as @posva said

[STUkh]

@posva, @jacekkarczmarczyk, @Kingwl, Morning. I've re-implemented exactly as @posva proposed before.

Now we expected that
@click="test" => click: _vm.test
@click.prevent="test" =>

click: function($event) {
    $event.preventDefault()
    return _vm.test($event)
}

@click.prevent="test1();test2()" =>

click: function($event) {
    $event.preventDefault()
    _vm.test1()
    _vm.test2()
}

[posva]

You probably need to return here as well and adapt other test cases

[STUkh]

@posva checked this case:
@click.prevent="(e) => test1(e)" compiled to:

click: function($event) {
                $event.preventDefault()
                ;(function(e) {
                  return _vm.test1(e)
                })($event)
              }

[STUkh]

@posva pushed change. Now ^ example returns

click: function($event) {
                $event.preventDefault()
                return (function(e) {
                  return _vm.test1(e)
                })($event)
              }

[Kingwl]

add some other test case for method path and function expression please

[STUkh]

@Kingwl there are 7 function expression tests was there. I've added more cases with methods and so on.

[Kingwl]

split(';') is not a good chooice