-
Notifications
You must be signed in to change notification settings - Fork 86
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
Change match order to avoid inefficient comparisons #114
Comments
I assume, similar to CSS handling in browsers, it's not optimized for individual searches, but for checking which nodes of AST matches possibly large set of selectors: https://stackoverflow.com/a/5813672 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I encountered the problem #111 today, where ESLint crashes due to TypeScript node types that are unrecognized by esquery. But it raises a different question about query strategy:
In my case, the selector
Program > :first-child
is trying to match the very first node in the AST tree. I was surprised to find that the crash was happening when this code is trying to find the:first-child
of aTSTypeAnnotation
very deep in the AST tree:esquery/esquery.js
Lines 301 to 317 in e27e73d
Why should this comparison be performed at all? I expected for
Program
to trivially fail to matchTSTypeAnnotation
, and the:first-child
would never even get tested.It seems that the query evaluation for
>
('child'
) compares the right side:first-child
BEFORE comparing the left sideProgram
:esquery/esquery.js
Lines 131 to 135 in e27e73d
Is there a good reason for that?
Logically it is like: "Is there a tire? --> Is the tire on a wheel? --> Is the wheel on a vehicle? --> Is the vehicle a motorcycle?"
Wouldn't it be better to first check for a motorcycle, before we go inspecting every tire?
The error goes away if I reorder the tests like this:
Should we make this change? Is there any downside?
The text was updated successfully, but these errors were encountered: