ESLint: Add bare token check to no-unknown-ds-tokens#76210
Conversation
The build-time fallback injection only processes tokens inside var() calls. Bare token references silently miss fallback injection. Add a bareToken check so the rule flags --wpds-* tokens not wrapped in var().
no-unknown-ds-tokens
|
Size Change: 0 B Total Size: 8.75 MB ℹ️ View Unchanged
|
|
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. |
|
Flaky tests detected in ec82b42. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23210747034
|
There was a problem hiding this comment.
Pull request overview
This PR enhances the @wordpress/no-unknown-ds-tokens ESLint rule to also flag bare --wpds-* token strings that are not wrapped in var(), aligning linting with the build-time fallback injection behavior (which only processes var(--wpds-*) patterns).
Changes:
- Add
extractVarWrappedTokens()helper and a newbareTokenreport to requirevar(--wpds-*)wrapping. - Exempt object property keys (custom property declarations) from the new bare-token check.
- Extend unit tests, documentation, and changelog to cover/enforce the new behavior.
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-unknown-ds-tokens.js | Adds var()-wrapping detection and reports bareToken violations (with a property-key exemption). |
| packages/eslint-plugin/rules/tests/no-unknown-ds-tokens.js | Adds/updates tests to validate new bareToken behavior and the key exemption. |
| packages/eslint-plugin/docs/rules/no-unknown-ds-tokens.md | Documents the new requirement that tokens must be var()-wrapped to receive fallbacks. |
| packages/eslint-plugin/CHANGELOG.md | Notes the enhancement in the Unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -98,10 +125,18 @@ module.exports = /** @type {import('eslint').Rule.RuleModule} */ ( { | |||
| } | |||
| } | |||
|
|
|||
| const varWrapped = extractVarWrappedTokens( | |||
| value, | |||
| DS_TOKEN_PREFIX | |||
| ); | |||
|
|
|||
| for ( const token of tokens ) { | |||
| if ( ! knownTokens.has( token ) ) { | |||
| unknownTokens.push( token ); | |||
| } | |||
| if ( ! varWrapped.has( token ) ) { | |||
| bareTokens.push( token ); | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
In the dynamic TemplateLiteral handler, unknownTokens / bareTokens are accumulated in arrays and can include duplicates when the same token appears in multiple quasis (e.g. before and after an expression). This can produce repeated entries in tokenNames and noisy/non-deterministic messages. Consider tracking these as Sets (and only formatting to a list at report time) to keep diagnostics unique and stable.
There was a problem hiding this comment.
Skipping this one since the same pattern already existed for unknownTokens before this PR. Could be a separate cleanup if it ever becomes a problem.
- Suppress bareToken for unknown tokens (already reported as onlyKnownTokens) to avoid conflicting guidance. - Replace extractVarWrappedTokens with extractBareTokens so the same token appearing both wrapped and bare in one string is correctly flagged.
aduth
left a comment
There was a problem hiding this comment.
LGTM 👍 Left a couple minor comments.
| // Skip bare-token check for property keys | ||
| // (e.g. `{ '--wpds-token': value }` declaring a custom property). | ||
| const isPropertyKey = | ||
| typeof node.value === 'string' && |
| * @param {string} [prefix=''] - Optional prefix to filter variables. | ||
| * @return {Set<string>} Tokens that are NOT wrapped in `var()`. | ||
| */ | ||
| function extractBareTokens( value, prefix = '' ) { |
There was a problem hiding this comment.
I feel like there might be some opportunity to collapse these two functions as they do a lot of the same thing, and potentially some overlap / simplification with how we use wpdsTokensRegex. This could prevent scanning the string multiple times, and in the case of bare tokens sometimes scanning unnecessarily (} else if ( bare.has( token ) ) { not guaranteed to ever be called).
But I see the challenge in trying to make sure it only returns unique values. e.g. my naive approach below works, but doesn't deduplicate.
> [...'--wpds-foo var(--wpds-baz) --wpds-bar'.matchAll(/(?:^|[^\\w])(var\(\s*)?(--wpds-[a-z-]+)/g)].map(([,varMatch,token])=>({ token, bare: !varMatch}))
//[
// { token: '--wpds-foo', bare: true },
// { token: '--wpds-baz', bare: false },
// { token: '--wpds-bar', bare: true }
//]|
Confirmed that there are no deprecated Eslint APIs being used (#76507). |
Collapse extractCSSVariables and extractBareTokens into a single classifyTokens function that categorises each token occurrence in one regex sweep. Also drop the redundant typeof guard from the isPropertyKey check.
# Conflicts: # packages/eslint-plugin/CHANGELOG.md
Follow-up to #75905
What?
The
no-unknown-ds-tokensESLint rule now reports an error when--wpds-*tokens are used as bare strings withoutvar()wrapping.Why?
The build-time fallback injection plugins (#75589) use the regex
var(\s*(--wpds-[\w-]+)\s*)to find and inject fallback values. This means only tokens insidevar()calls receive fallbacks — bare references like'--wpds-color-fg-default'are silently skipped.This gap was surfaced by the Stack component refactor (#75589), where the token map had to be changed from bare strings to
var()calls for fallbacks to work:The existing rule already checked token validity and flagged dynamic construction, but didn't enforce
var()syntax. This meant a valid token used withoutvar()would pass linting while silently missing fallback injection.How?
extractVarWrappedTokens()helper that finds tokens appearing as directvar()arguments (handles nested calls likevar(--a, var(--b))).bareTokenmessage reported for any--wpds-*token not insidevar(), in both the static string and dynamic template literal handlers.{ '--wpds-token': value }) are exempted, since those are custom property declarations and are handled by a separate rule (no-setting-ds-tokens). An accompanying enhancement for that is at ESLint: Broadenno-setting-ds-tokensto all object property keys #76212.Testing Instructions
Run the rule's unit tests:
You can also test manually by adding a bare token to a component file under the rule's scope.
'--wpds-color-fg-content-neutral'should error, while'var(--wpds-color-fg-content-neutral)'should pass.