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

Enable support for eslint v7 #25

Merged
merged 4 commits into from
Apr 5, 2023
Merged

Enable support for eslint v7 #25

merged 4 commits into from
Apr 5, 2023

Conversation

chawes13
Copy link
Contributor

@chawes13 chawes13 commented Apr 4, 2023

Resolves #24

Installing eslint@^8.0 as a dependency was causing some issues when trying to use this configuration on projects with eslint@^7.0. This makes it impossible to update the configuration on older projects without also requiring an update to eslint@^8.

There is still a breaking change, which is the requirement to use babel.config.js instead of .babelrc for the parser to work correctly. I'll update the release notes with this information accordingly.

@chawes13 chawes13 requested a review from mwislek April 4, 2023 18:38
@chawes13 chawes13 changed the title Hotfix/update dependencies Enable support for eslint v7 Apr 4, 2023
Copy link

@mwislek mwislek left a comment

Choose a reason for hiding this comment

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

Not sure I totally follow "eslint": "^8.0.0" as a "devDependency" and "eslint": "^7.5.0 || ^8.0.0" as a "peerDependency". I think the "peer" makes sense: the including dependent is expected to have a version of eslint installed at either 7.5 or 8 (is there a reason this isn't >=7.5?) I don't understand the "devDependency" though.

Also, why is @babel/core a "peerDependency" when this module only seems to use @babel/core for its own "dev" purposes.

@chawes13
Copy link
Contributor Author

chawes13 commented Apr 5, 2023

Not sure I totally follow "eslint": "^8.0.0" as a "devDependency" and "eslint": "^7.5.0 || ^8.0.0" as a "peerDependency". I think the "peer" makes sense: the including dependent is expected to have a version of eslint installed at either 7.5 or 8 (is there a reason this isn't >=7.5?) I don't understand the "devDependency" though.

eslint is required as a dev dependency because we're using eslint in our tests!

Also, why is @babel/core a "peerDependency" when this module only seems to use @babel/core for its own "dev" purposes.

@babel/core is a peer dependency of @babel/eslint-parser. Since this package isn't requiring them as dependencies, we're passing that requirement on to the consumers of this package. Maybe we don't need to do that since it'll get picked up when installing @babel/eslint-parser? It felt more communicative to me at the time, but doesn't make much sense now! Thanks for flagging that.

@chawes13 chawes13 merged commit 0361890 into main Apr 5, 2023
@chawes13 chawes13 deleted the hotfix/update-dependencies branch April 5, 2023 15:43
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.

Explore dependency recategorization
2 participants