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

[utils] [patch] Fix @babel/eslint-parser 8 compatibility #2343

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

nicolo-ribaudo
Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo commented Dec 30, 2021

In Babel 8, you won't be able to directly require @babel/eslint-parser/worker/ast-info.cjs anymore (there is a reason if the path contains "worker": it's meant to be used in workers after properly setting them up 😛). However, @babel/eslint-parser exposes the visitorKeys on its result object, similarly to how @typescript-eslint/parser does: you can access it without requiring any internal file.

We caught this in our integration tests: even if for some reason the error is logged without reporting a failure, in https://app.circleci.com/pipelines/github/babel/babel/8764/workflows/3516a741-7bca-4713-8b12-9f2f20310066/jobs/50483/parallel-runs/0/steps/0-103 you can see

Error while parsing /home/circleci/babel/eslint/babel-eslint-tests/test/fixtures/eslint-plugin-import/a.js
Line undefined, column undefined: Cannot read properties of undefined (reading 'VISITOR_KEYS')
 PASS  eslint/babel-eslint-tests/test/integration/eslint-plugin-import.js (66.694 s)

There aren't any tests for @babel/eslint-parser, and I'm not sure about how to add them from scratch because it requires a babel.config.json file when used with eslint-plugin-import (to specify which additional syntax should be supported, such as TS). However, I verified manually that this change makes our test pass.

utils/parse.js Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 31, 2021

Codecov Report

Merging #2343 (210e40a) into main (e3ca68e) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2343      +/-   ##
==========================================
+ Coverage   94.72%   94.85%   +0.13%     
==========================================
  Files          65       65              
  Lines        2690     2684       -6     
  Branches      893      890       -3     
==========================================
- Hits         2548     2546       -2     
+ Misses        142      138       -4     
Impacted Files Coverage Δ
utils/parse.js 91.30% <100.00%> (+6.68%) ⬆️

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 e3ca68e...210e40a. Read the comment docs.

@ljharb ljharb added the package: utils eslint-module-utils package label Jan 1, 2022
@ljharb ljharb changed the title Fix @babel/eslint-parser 8 compatibility [utils] [patch] Fix @babel/eslint-parser 8 compatibility Jan 1, 2022
@ljharb ljharb merged commit 210e40a into import-js:main Jan 1, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the babel-8 branch January 1, 2022 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: utils eslint-module-utils package
Development

Successfully merging this pull request may close these issues.

3 participants