Eslint Plugin: Change plugin dependencies to peer dependencies #42656
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ch recommendations of shareable configs in Eslint. For reference https://eslint.org/docs/latest/developer-guide/shareable-configs#publishing-a-shareable-config
BE-Webdesign
requested review from
gziolo,
ntwb,
nerrad and
ajitbohra
as code owners
|
Size Change: 0 B Total Size: 1.26 MB ℹ️ View Unchanged
|
…ies of main project.
gziolo
added
[Package] ESLint plugin
|
What's missing here? Using Node 18 becomes problematic now. This is trying to use a version of But it'd be great if there was a less hacky way of dealing with this. |
|
@BE-Webdesign Are you able to update this branch and resolve the merge conflicts that are listed? |
1 task
ntwb
requested changes
Nov 8, 2022
There was a problem hiding this comment.
This approach as it stands currently won't work unfortunately and will break @wordpress/scripts package:
This is because the ESLint dependencies are deliberately included in the package so that wp-scripts lint-js will work straight out-of-the-box with the included WordPress JavaScript Coding Standards ESLint configuration included
There's potentially some additional work that might make switching to peerDependencies work as part of the @wordpress/scripts package
This would also be a major breaking change for the @wordpress/eslint-plugin & @wordpress/scripts packages and would need further changes to facilitate this
|
Following up from my above code review notes, I don't think this PR is what will resolve the 10Up issue you're facing either I think what is actually required to solve the linked issue is to update gajus/eslint-plugin-jsdoc@v39.2.4...v39.2.5 Instructions on how to update packages in this repo are here if you're interested in updating https://github.com/WordPress/gutenberg/tree/trunk/packages#managing-packages |
|
@fabiankaegy unfortunately, I have not been able to keep up with GitHub lately. I believe as @ntwb has pointed out although this may solve some issues, it creates others and a different approach will be needed so resolving the merge conflicts will likely not be satisfactory to solve the issues. I currently do not have the bandwidth to properly work on the issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #39810
What?
Change dependencies to match recommendations of shareable configs in Eslint. For reference https://eslint.org/docs/latest/developer-guide/shareable-configs#publishing-a-shareable-config
Why?
Avoid potential versioning conflicts when package is used in other projects, or if Gutenberg introduces new packages related to eslint.
How?
Moves the eslint plugin dependencies into peer dependencies using the >= range operator.
Testing Instructions
node_modulesfolder.npm ito install deps.npm run lint-jsto ensure that all plugins for eslint are still loading correctly, you should see warnings still.Screenshots or screencast
Not applicable