Skip to content

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Apr 23, 2025

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Refactor

What changes did you make? (Give an overview)

This refactors the ESQuery selector evaluation done by NodeEventGenerator. Previously, it would traverse the selector AST three times to collect different data. This refactor combines those three traversals into one to avoid extra work.

  • Split out ESQuery logic into its own file (necessary as part of core rewrite work to avoid duplication)
  • Added tests for ESQuery parsing and matching
  • Refactored logic into a single-pass traversal

Is there anything you'd like reviewers to focus on?

@nzakas nzakas requested a review from a team as a code owner April 23, 2025 14:48
@eslint-github-bot eslint-github-bot bot added the chore This change is not user-facing label Apr 23, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Apr 23, 2025
@github-actions github-actions bot added the core Relates to ESLint's core APIs and features label Apr 23, 2025
Copy link

netlify bot commented Apr 23, 2025

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 4b1b5b2
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/680be7fb8e46af00086a2c80

@nzakas nzakas changed the title refactor: One-shot ESQUer selector analysis refactor: One-shot ESQuery selector analysis Apr 23, 2025
@nzakas nzakas marked this pull request as draft April 23, 2025 15:05
@nzakas nzakas marked this pull request as ready for review April 23, 2025 17:51
@mdjermanovic mdjermanovic moved this from Needs Triage to Implementing in Triage Apr 25, 2025
@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Apr 25, 2025
nzakas and others added 5 commits April 25, 2025 15:50
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 40dd299 into main Apr 26, 2025
31 checks passed
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Apr 26, 2025
@mdjermanovic mdjermanovic deleted the specificity-perf branch April 26, 2025 16:26
@fisker
Copy link
Contributor

fisker commented Apr 29, 2025

Can we expose this, so that plugins can use?

eslint-plugin-unicorn use esquery to allow user config with selectors https://github.com/sindresorhus/eslint-plugin-unicorn/blob/3838ec815057154a7fb4cd8257abfb554502ba2f/rules/template-indent.js#L29-L35

@nzakas
Copy link
Member Author

nzakas commented Apr 29, 2025

@fisker please open a feature request so we can understand what it is you're asking for.

@fisker
Copy link
Contributor

fisker commented Apr 30, 2025

Sorry, read it to quickly, I thought esquery was replaced. Deeply sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing core Relates to ESLint's core APIs and features
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

3 participants