ESLint: Broaden no-setting-ds-tokens to all object property keys#76212
ESLint: Broaden no-setting-ds-tokens to all object property keys#76212
no-setting-ds-tokens to all object property keys#76212Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
no-setting-ds-tokens to all object property keys
|
Size Change: 0 B Total Size: 8.75 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 4dd81e0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23201685654
|
There was a problem hiding this comment.
Pull request overview
Updates the @wordpress/no-setting-ds-tokens ESLint rule to catch --wpds-* CSS custom property object keys beyond JSX style attributes, closing a linting gap for style objects declared outside JSX.
Changes:
- Broadens the rule’s AST selector to report
--wpds-*string-literal property keys anywhere in the file. - Extends unit tests to cover non-JSX object literals and updates rule documentation accordingly.
- Updates eslint config overrides and plugin changelog to reflect the broadened behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/eslint-plugin/rules/no-setting-ds-tokens.js | Removes the JSX style-attribute ancestor constraint from the selector. |
| packages/eslint-plugin/rules/tests/no-setting-ds-tokens.js | Adds valid/invalid cases for --wpds-* keys in plain object literals. |
| packages/eslint-plugin/docs/rules/no-setting-ds-tokens.md | Updates rule scope description and examples to include non-JSX usage. |
| packages/eslint-plugin/CHANGELOG.md | Notes the enhancement in Unreleased changelog. |
| .eslintrc.js | Disables the rule within packages/eslint-plugin/** and packages/theme/**. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I delegated this review to Copilot as this feels like exactly the task it would be quite good at. |
ciampo
left a comment
There was a problem hiding this comment.
LGTM 🚀
I believe that the new rule still wouldn't catch Map or computed property patterns like { [tokenVar]: 'red' } where the key is a variable holding a --wpds-* string — but that's inherently a limitation of static analysis and not a concern for this PR.
|
Confirmed that there are no deprecated Eslint APIs being used (#76507). |
Companion to #76210
What?
The
no-setting-ds-tokensESLint rule now flags--wpds-*property keys in any object, not just inside JSXstyleattributes.Why?
The rule previously only matched property keys inside
JSXAttribute[name.name="style"], so tokens defined in plain objects or variables were invisible to it:Same gap that
no-unknown-ds-tokenshad before #75905 broadened it.The broadening should be safe. The selector
Property[key.value=/^--wpds-/]only fires on string-literal property keys starting with--wpds-, which in practice only appear in style objects. There's no realistic JS pattern where someone would use a--wpds-*string as an object key for a non-CSS purpose.How?
Removed the
JSXAttribute[name.name="style"] ObjectExpression >ancestor constraint from the AST selector, leaving justProperty[key.value=/^--wpds-/].Testing Instructions
Run the rule's unit tests: