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

Fix: Crash with esquery when using JSX (fixes #13639) #14072

Merged
merged 3 commits into from Feb 12, 2021
Merged

Fix: Crash with esquery when using JSX (fixes #13639) #14072

merged 3 commits into from Feb 12, 2021

Conversation

ota-meshi
Copy link
Member

Prerequisites checklist

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

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

fixes #13639

What changes did you make? (Give an overview)

This PR uses a new option in esquery to solve the crash problem with non-standard ASTs such as JSX and some queries.

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

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Feb 6, 2021
@ota-meshi
Copy link
Member Author

I think there is an error in the test that has nothing to do with my changes. Can I include a fix for the error in this PR?

@ota-meshi
Copy link
Member Author

I think it might be fixed in #14067.

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint upgrade This change is related to a dependency upgrade and removed triage An ESLint team member will look at this issue soon labels Feb 7, 2021
@mdjermanovic
Copy link
Member

Hi @ota-meshi, thanks for this PR, and thanks for working on estools/esquery#112!

I think there is an error in the test that has nothing to do with my changes.

We'll try to fix that as soon as possible, once we figure out what's the best course of action (probably #14067).

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for doing all the work to make this possible, @ota-meshi!

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.

The code looks good, I left some questions about testing.

lib/linter/node-event-generator.js Show resolved Hide resolved
lib/linter/node-event-generator.js Show resolved Hide resolved
lib/linter/linter.js Show resolved Hide resolved
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 for the great work!

@mdjermanovic mdjermanovic merged commit 9d6063a into eslint:master Feb 12, 2021
@mdjermanovic
Copy link
Member

Thanks for contributing!

This was referenced Mar 17, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 12, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint upgrade This change is related to a dependency upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exceptions when traversing using sibling, adjacent and nth-child selectors and non-standard AST nodes
3 participants