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

Checking whether v-on handler is a function invocation is broken #11894

Open
adamsol opened this issue Jan 27, 2021 · 2 comments
Open

Checking whether v-on handler is a function invocation is broken #11894

adamsol opened this issue Jan 27, 2021 · 2 comments

Comments

@adamsol
Copy link

adamsol commented Jan 27, 2021

Version

2.6.11

Reproduction link

https://jsfiddle.net/adamsol/pknr8dae/

Steps to reproduce

Click the buttons.

What is expected?

All the buttons should behave in the same way: a message should appear below.

What is actually happening?

Only the first button works correctly.


See #11893 for the origin of the issue.

The problem lies probably here:

var isFunctionInvocation = simplePathRE.test(handler.value.replace(fnInvokeRE, ''));

The regexes used do not take into account cases such as additional spaces, parentheses, or chained function invocations. As a result, a promise is correctly returned only in the first case in the repro, and in all the other cases errorHandler won't capture the exception thrown in the async method.

The difference in the generated code (return is present only in the first case): https://template-explorer.vuejs.org/#%3Cdiv%20id%3D%22app%22%3E%0A%20%20%3Cbutton%20%40click%3D%22click(1)%22%3E%0A%20%20%20%20click(1)%0A%20%20%3C%2Fbutton%3E%0A%20%20%3Cbutton%20%40click%3D%22click%20(2)%22%3E%0A%20%20%20%20click%20(2)%0A%20%20%3C%2Fbutton%3E%0A%20%20%3Cbutton%20%40click%3D%22click((3))%22%3E%0A%20%20%20%20click((3))%0A%20%20%3C%2Fbutton%3E%0A%20%20%3Cbutton%20%40click%3D%22(click(4))%22%3E%0A%20%20%20%20(click(4))%0A%20%20%3C%2Fbutton%3E%0A%20%20%3Cbutton%20%40click%3D%22click(5).then()%22%3E%0A%20%20%20%20click(5).then()%0A%20%20%3C%2Fbutton%3E%0A%3C%2Fdiv%3E

Suggested solution: either add return in every case, or don't add it at all, so that the behaviour is consistent. If checking for the function invocation is crucial, then the code must be parsed in some other way.

@posva
Copy link
Member

posva commented Jan 27, 2021

See #7628

Handling all possible cases will require a parser instead of a regex but realistically speaking, people won't write @click="method((2))". The only that would be worth adding support for is @click="click(5).catch(() => {})" but it has the same problem, it requires a full parsing to deduce make it fully consistent. You can wrap the call with a function: () => method().catch(() => {}) or add a method to your component, which is prefered in such scenarios because the code becomes difficult to read. The same for more complicated expressions where parentheses are required, like mathematical expressions.

That being said, Vue 3 does support these syntaxes but it has a full parser built in it. So maybe someone finds a way to improve the existing regex

@adamsol
Copy link
Author

adamsol commented Feb 23, 2021

The documentation (https://vuejs.org/v2/api/#errorHandler) says:

In 2.6.0+, [...] if any of the covered hooks or handlers returns a Promise chain (e.g. async functions), the error from that Promise chain will also be handled.

So I think the current behaviour should be considered a bug, since v-on is one of the covered hooks, .then and .catch create promise chains, but errors are not handled. Also, the behaviour is inconsistent even between method() and method (), which is very surprising. Together with #10009, this makes errorHandler hardly usable with regard to async methods.


For anyone who stumbles upon this issue: to catch all errors in promises, use unhandledrejection event as described here: https://stackoverflow.com/a/52076738. Note that you still need to set up Vue.config.errorHandler, since in the default handler Vue silences the errors that it manages to catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants