Conversation
|
Size Change: 0 B Total Size: 6.84 MB ℹ️ View Unchanged
|
ciampo
reviewed
Feb 24, 2026
Contributor
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
Review: ESLint no-ds-tokens rule
Nice, clean rule. The implementation is minimal and well-structured, and the use of ESLint AST selectors is consistent with the sibling no-unknown-ds-tokens rule. Here are a few observations:
1. Regex edge case: strings starting with --wpds-
The regex [^\w]--wpds- requires a non-word character before --wpds-. This means a string that starts directly with --wpds- won't be matched:
// NOT caught by the rule:
element.style.setProperty( '--wpds-color-fg', value );
const token = '--wpds-color-fg';This is consistent with the existing no-unknown-ds-tokens rule (which uses the same pattern), and the primary use case (var(--wpds-...)) is covered. But if you wanted to also catch the bare --wpds-* patterns, the regex could be amended to something like:
const wpdsTokensRegex = new RegExp( `(^|--)${ DS_TOKEN_PREFIX }|[^\\w]--${ DS_TOKEN_PREFIX }`, 'i' );Or more simply, if the intent is to catch any mention:
const wpdsTokensRegex = /--wpds-/i;This is likely fine as-is for the stated purpose (preventing tokens in Emotion CSS strings), but might be worth either (a) noting the limitation in the doc, or (b) adding a valid-case test to document the intentional gap. Up to you.
2. Missing CHANGELOG entry
The ## Unreleased section in packages/eslint-plugin/CHANGELOG.md is currently empty. Based on the convention from v24.0.0 (which introduced the sibling DS token rules), a ### New Features entry would be expected:
## Unreleased
### New Features
- Added [`no-ds-tokens`](https://github.com/WordPress/gutenberg/blob/HEAD/packages/eslint-plugin/docs/rules/no-ds-tokens.md) rule to disallow usage of Design System token CSS custom properties (`--wpds-*`).3. Test coverage — multiple tokens only expecting one error
The test case with var(--wpds-border-color, var(--wpds-border-fallback)) expects a single error, which is correct since both tokens live in the same TemplateElement AST node. This is fine, but if you wanted to call out which tokens were found (like no-unknown-ds-tokens does with its tokenNames data), it could be helpful for debugging. Not a blocker — just a thought since the semantics of this rule ("all tokens are banned") make individual enumeration less critical.
Overall this looks good and ready to go pending the CHANGELOG addition and whatever you decide about the regex edge case. 👍
|
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. |
Contributor
|
@mirka I was curious to see what it would be like to delegate a PR review ⬆️ |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new @wordpress/no-ds-tokens ESLint rule to prevent --wpds-* Design System CSS custom properties from being introduced in JS/TS string/template literals, and enables the rule for packages/components/src (excluding tests/stories) to avoid token usage in contexts where fallback injection can’t run (e.g., Emotion).
Changes:
- Added a new
@wordpress/no-ds-tokensrule that reports--wpds-*occurrences inLiteralandTemplateElementnodes. - Added unit tests and rule documentation.
- Enabled the rule via an override scoped to
packages/components/src/**(excludingtest/andstories/).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/eslint-plugin/rules/no-ds-tokens.js | Implements the new ESLint rule that detects --wpds-* tokens in literals/templates. |
| packages/eslint-plugin/rules/tests/no-ds-tokens.js | Adds RuleTester coverage for allowed/disallowed examples. |
| packages/eslint-plugin/docs/rules/no-ds-tokens.md | Documents the rule and provides examples. |
| .eslintrc.js | Enables the rule for packages/components/src/** excluding tests/stories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ciampo
approved these changes
Feb 25, 2026
Contributor
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
LGTM 🚀
None of the remaining feedback feels blocking
This file contains hidden or 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
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.
What?
Adds a new
@wordpress/no-ds-tokensESLint rule that disallows any usage of--wpds-*CSS custom properties in string literals and template literals.Enables it in our repo for
packages/components/srconly.Why?
The design token fallback build plugins (#75589) can't inject fallbacks into Emotion files due to a conflict between the esbuild token fallback plugin's
onLoadhook and the Emotion Babel plugin — only one can handle a given file. This means any--wpds-*tokens used in Emotion styles would render without fallback values, breaking in environments where the@wordpress/themestylesheet isn't available.This lint rule prevents tokens from being introduced into code that the fallback plugins can't reach. Of course, it could also be used when you want to ban design tokens in certain files, for whatever reason.
In terms of the eslint config for our repo, we target all non-story and non-test files in the
@wordpress/componentspackage. Technically, it's safe to use them in JS files as long as they don't include Emotion, but here we ban them categorically for simplicity.How?
@wordpress/eslint-pluginmatching--wpds-*patterns inLiteralandTemplateElementAST nodes. Added to the shared package rather than as a repo-local rule since it could be useful for third-party consumers in specific contexts as well.packages/eslint-plugin/docs/rules/no-ds-tokens.md(not listed in the README yet since DS tokens aren't publicly documented)..eslintrc.jsscoped topackages/components/src/**, excluding tests and stories.Testing Instructions
npm run test:unit -- packages/eslint-plugin/rules/__tests__/no-ds-tokens.js --no-coverage— all tests should pass.color: var(--wpds-color-fg-content-neutral)to a template literal in any file underpackages/components/src/and confirm that ESLint reports an error.