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

[Tests] use @babel/eslint-parser instead of babel-eslint #811

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

MichaelDeBoey
Copy link
Contributor

babel-eslint is deprecated in favor of @babel/eslint-parser

https://babeljs.io/blog/2020/07/13/the-state-of-babel-eslint

@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 30, 2021
12 tasks
@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #811 (1f5416d) into master (0021489) will not change coverage.
The diff coverage is n/a.

❗ Current head 1f5416d differs from pull request most recent head aed7a20. Consider uploading reports for the commit aed7a20 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #811   +/-   ##
=======================================
  Coverage   99.24%   99.24%           
=======================================
  Files          98       98           
  Lines        1453     1453           
  Branches      477      477           
=======================================
  Hits         1442     1442           
  Misses         11       11           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0021489...aed7a20. Read the comment docs.

@fgblomqvist
Copy link

Anything blocking this from getting merged?

@MichaelDeBoey
Copy link
Contributor Author

Only a review from @ljharb

package.json Show resolved Hide resolved
@ljharb ljharb changed the title chore: use @babel/eslint-parser instead of babel-eslint [Tests] use @babel/eslint-parser instead of babel-eslint Oct 12, 2021
@ljharb
Copy link
Member

ljharb commented Oct 12, 2021

also, the word "chore" is very inaccurate; chores are burdensome and changes like this are helpful.

@MichaelDeBoey MichaelDeBoey requested a review from ljharb October 14, 2021 12:11
@gajus
Copy link

gajus commented Oct 25, 2021

@ljharb Did you want the commit to be renamed to feat:?

@ljharb
Copy link
Member

ljharb commented Oct 26, 2021

nah, conventional commits are garbage :-) i just haven't gotten back to it since the last update.

@ljharb ljharb force-pushed the update-babel-eslint-package branch 2 times, most recently from 8b21917 to 1cdeb11 Compare October 26, 2021 01:38
@ljharb
Copy link
Member

ljharb commented Oct 26, 2021

I think because the parser added an engines declaration in v7.11 that drops node 11, npm selects eslint 6 to match, and then that fails the peer dep requirement. Let's see if this does it.

@ljharb ljharb force-pushed the update-babel-eslint-package branch from 1cdeb11 to f2a3950 Compare October 26, 2021 01:52
@ljharb ljharb force-pushed the update-babel-eslint-package branch 3 times, most recently from 476d5f8 to aed7a20 Compare October 26, 2021 02:17
@ljharb
Copy link
Member

ljharb commented Oct 26, 2021

k, turns out all versions of babel's eslint parser have a peer dep on eslint 7, which has an engines that doesn't include node 11, so that's the issue here. alternatively we could stop testing eslint 7 on node 11, which i'll probably just do in a followup commit.

@ljharb ljharb merged commit aed7a20 into jsx-eslint:master Oct 26, 2021
@gajus
Copy link

gajus commented Oct 26, 2021

🎉

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

Successfully merging this pull request may close these issues.

4 participants