Eslint: update WP eslint and add new components-no-missing-40px-size-prop#46640
Eslint: update WP eslint and add new components-no-missing-40px-size-prop#46640simison wants to merge 2 commits into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
This PR updates the WordPress ESLint plugin from version 23.0.0 to 24.0.0 and enables a new ESLint rule @wordpress/components-no-missing-40px-size-prop with the checkLocalImports: true option.
Changes:
- Updates
@wordpress/eslint-plugindependency from 23.0.0 to 24.0.0 - Enables the new
components-no-missing-40px-size-proprule with local import checking - Updates pnpm-lock.yaml with all transitive dependency updates for the new ESLint plugin version
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/js-tools/package.json | Updates @wordpress/eslint-plugin from 23.0.0 to 24.0.0 |
| tools/js-tools/eslintrc/base.mjs | Adds configuration for the new components-no-missing-40px-size-prop rule with checkLocalImports option |
| pnpm-lock.yaml | Updates lock file with transitive dependencies for the new WordPress ESLint plugin version |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| // Replacement for throttle is not as straightforward as you-dont-need-lodash-underscore claims. | ||
| 'you-dont-need-lodash-underscore/throttle': 'off', | ||
|
|
||
| '@wordpress/components-no-missing-40px-size-prop': 'error', |
There was a problem hiding this comment.
We could start with warning too.
There was a problem hiding this comment.
This is not the right place to add this. This is a configuration block specific to the you-dont-need-lodash-underscore package.
Warning or error doesn't matter. I tend towards a philosophy of "non-required linter warnings are fairly pointless because people will ignore them", so our CI requires warnings to be fixed as well as errors.
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
721e5dd to
5e429f4
Compare
| // Replacement for throttle is not as straightforward as you-dont-need-lodash-underscore claims. | ||
| 'you-dont-need-lodash-underscore/throttle': 'off', | ||
|
|
||
| '@wordpress/components-no-missing-40px-size-prop': 'error', |
There was a problem hiding this comment.
This is not the right place to add this. This is a configuration block specific to the you-dont-need-lodash-underscore package.
Warning or error doesn't matter. I tend towards a philosophy of "non-required linter warnings are fairly pointless because people will ignore them", so our CI requires warnings to be fixed as well as errors.
| }, | ||
| }, | ||
| }, | ||
| rules: { |
There was a problem hiding this comment.
At the start of this block (since "@wordpress/c" sorts before the existing "@wordpress/i" rule) would be more appropriate.
| resolution: {integrity: sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd3iBHlynerhQ7BhCkn2BPbQUL+rGqFg==} | ||
| engines: {node: '>=6.9.0'} | ||
|
|
||
| '@babel/code-frame@7.28.6': |
There was a problem hiding this comment.
A run of pnpm dedupe would be nice to avoid all these extra package versions.
Even better, since the linting rule isn't enabled by default, I think I'll tell Renovate to go ahead with the update of the whole @wordpress/* monorepo (which will include this package), and then this PR can be a follow-up to enable the new rule.
|
This PR has been marked as stale. This happened because:
If this PR is still useful, please do a [trunk merge or rebase](https://github.com/Automattic/jetpack/blob/trunk/docs/git-workflow.md#keeping-your-branch-up-to-date) and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation. If the PR is not updated (or at least commented on) in another month, it will be automatically closed. |
|
This PR has been automatically closed as it has not been updated in some time. If you want to resume work on the PR, feel free to restore the branch and reopen the PR. |
WIP: didn't fix errors yet, current status:
Proposed changes:
@wordpress/components-no-missing-40px-size-propruleRule was added in WordPress/gutenberg#74622
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
pnpm run lintmanually__next40pxDefaultSizeprop), make sure that the linter correctly reports it