-
Notifications
You must be signed in to change notification settings - Fork 9
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
simpleComputed regex edge case #358
Comments
Thanks for the suggestion. As I try to avoid using a parser, I should check if we really need it here. |
@rrd108 I played around with this issue (as it comes from Reddit) and when there are multiple conditions inside the computed property it indeed gets weird and the regex start to become more complex. I didnt try it but something that might work is changing the logic to not use regex either but just find open and close brackets and do the magic (something we have in some rules already like the |
Yes. That was the one. |
I'm curious why this is? Regex is the wrong tool for the job here and a parser/AST is the right tool. I'm not trying to be rude and ofc you can do what you want with your project, I'm just a bit confused why creating maintainability issues, reliability issues, and edge case tradeoffs when you could avoid all of it |
We just missing some benchmarking to check it. If you can do some for us you are very welcome to do so! |
Esprima seems to be a bteer option then babel. |
Esprima doesn't support typescript syntax as far as I know. Babel does and it can generate an ESTree-compliant AST that can be used with various libraries like ast-types and esquery not like SWC which has its own non-ESTree AST and isn't compatible with the myriad of libraries for working with ESTree ASTs Another option is to use ast-grep API which has its own vue language support |
Yes, I saw that. I created a dedicated issue for this. see #368 |
In this issue the thing is this block should be reported or not? @rrd108 @UnrefinedBrain const foo = computed(() => {
if (bar) {
if (baz) {
}
}
}); I did some work with babel parser for the |
Depends on what is inside the inner |
vue-mess-detector/src/rules/vue-strong/simpleComputed.ts
Line 17 in eec64e3
Regex is not able to reliably match the entire code block because it lacks the capability to match unknown numbers of balanced pairs. If there is more than one level of nesting, the current regex here will not match it
Example of issue (regex101 link):
highly suggest switching to an AST-based approach rather than regex for this. It might look like this:
The text was updated successfully, but these errors were encountered: