ESLint: Broaden no-unknown-ds-tokens to all strings and catch dynamic construction#75905
ESLint: Broaden no-unknown-ds-tokens to all strings and catch dynamic construction#75905
no-unknown-ds-tokens to all strings and catch dynamic construction#75905Conversation
| When using Design System tokens (CSS custom properties beginning with `--wpds-`), only valid public tokens should be used. Using non-existent tokens will result in broken styles since the CSS variable won't resolve to any value. | ||
|
|
||
| This rule lints JSX inline styles. For CSS files, use the [corresponding Stylelint rule](https://developer.wordpress.org/block-editor/reference-guides/packages/packages-theme/#stylelint-plugins) from the `@wordpress/theme` package. | ||
| Additionally, token names must not be dynamically constructed (e.g. via template literal expressions), as they cannot be statically verified for correctness or processed automatically to inject fallbacks. |
There was a problem hiding this comment.
I considered separating the two lint rules, but since a dynamically generated token will always evade an unknown tokens lint, I think it makes sense to keep the two lints as a single rule. The fallback injection thing is secondary — not necessarily the main motivator of this additional check.
|
Size Change: 0 B Total Size: 6.84 MB ℹ️ View Unchanged
|
|
|
||
| const knownTokens = new Set( tokenList ); | ||
| const wpdsTokensRegex = new RegExp( `[^\\w]--${ DS_TOKEN_PREFIX }`, 'i' ); | ||
| const wpdsTokensRegex = new RegExp( `(?:^|[^\\w])--${ DS_TOKEN_PREFIX }`, 'i' ); |
There was a problem hiding this comment.
Tweaked to match the "start of string" case raised in #75872 (review).
|
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. |
There was a problem hiding this comment.
Pull request overview
Updates the @wordpress/no-unknown-ds-tokens ESLint rule to catch invalid Design System tokens across all JS/TS strings (not just JSX style props) and to flag dynamically constructed --wpds-* token names which can’t be statically validated.
Changes:
- Expand rule matching to all string literals and template literals in JS/TS.
- Add a dedicated
dynamicTokenreport for template literals with expressions that include--wpds-. - Update docs/changelog and adjust repo ESLint config to disable the rule within
packages/eslint-plugin/**andpackages/theme/**.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/eslint-plugin/rules/no-unknown-ds-tokens.js | Broadens AST matching scope and introduces dynamic-token reporting. |
| packages/eslint-plugin/rules/tests/no-unknown-ds-tokens.js | Adds new valid/invalid cases for broader matching and dynamic detection. |
| packages/eslint-plugin/docs/rules/no-unknown-ds-tokens.md | Documents expanded scope and dynamic-token restriction. |
| packages/eslint-plugin/CHANGELOG.md | Notes the rule enhancement in Unreleased. |
| .eslintrc.js | Disables the rule for eslint-plugin and theme packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0] TemplateElement[value.raw=${ wpdsTokensRegex }]`; | ||
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; | ||
| return { | ||
| [ dynamicTokensAST ]( node ) { | ||
| context.report( { | ||
| node: node.parent, | ||
| messageId: 'dynamicToken', | ||
| } ); |
There was a problem hiding this comment.
dynamicTokensAST currently reports dynamicToken for any template literal that has expressions and contains a --wpds- segment in any quasi. This will also flag cases where the token itself is fully static (e.g. `var(--wpds-color-fg-content-neutral) ${suffix}`), even though the fallback injection regex can still process the static var(--wpds-...) substring. Consider tightening the detection to only report when the token name is actually interrupted by an expression (e.g. a quasi ends with a partial token like --wpds-...-), and add a regression test for the static-token-with-expression case.
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0] TemplateElement[value.raw=${ wpdsTokensRegex }]`; | ||
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; | ||
| return { | ||
| [ dynamicTokensAST ]( node ) { | ||
| context.report( { | ||
| node: node.parent, |
There was a problem hiding this comment.
Reporting on node.parent from the TemplateElement visitor can produce duplicate dynamicToken errors when a single template literal contains multiple --wpds- occurrences across multiple quasis (each matching TemplateElement[...]). Consider selecting/reporting the TemplateLiteral node directly (e.g. via a TemplateLiteral:has(...) selector) or tracking already-reported template literals to ensure only one report per template literal.
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0] TemplateElement[value.raw=${ wpdsTokensRegex }]`; | |
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; | |
| return { | |
| [ dynamicTokensAST ]( node ) { | |
| context.report( { | |
| node: node.parent, | |
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0]:has(TemplateElement[value.raw=${ wpdsTokensRegex }])`; | |
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; | |
| return { | |
| [ dynamicTokensAST ]( node ) { | |
| context.report( { | |
| node, |
| { | ||
| code: 'const token = `var(--wpds-dimension-gap-${ size })`;', | ||
| errors: [ | ||
| { | ||
| messageId: 'dynamicToken', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: '<div style={ { gap: `var(--wpds-dimension-gap-${ size })` } } />', | ||
| errors: [ |
There was a problem hiding this comment.
Add a valid test case for a template literal that contains an expression but where the --wpds-* token reference is fully static (e.g. const css = `var(--wpds-color-fg-content-neutral) ${ suffix }`;). This guards against over-reporting dynamicToken for template literals that the fallback injection logic can still safely process.
ciampo
left a comment
There was a problem hiding this comment.
The performance hit shouldn't be too bad (regexes are cheap and not many strings will match), and the benefits are clear — therefore, for me, this feels like a nice improvement.
Shouldn't we expect the lint to generate an error in the CI task re. the dynamic gap token in Stack?
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0] TemplateElement[value.raw=${ wpdsTokensRegex }]`; | ||
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; |
There was a problem hiding this comment.
The logic here could generate false positives with a confusing message: the user would be told "tokens must not be dynamically constructed" even when the token isn't dynamic.
If a template literal has any expression, every TemplateElement in it containing --wpds- matches dynamicTokensAST, even if the token is fully static and the expression is elsewhere in the string. For example:
const style = `color: ${color}; gap: var(--wpds-dimension-gap-sm)`;The template element "; gap: var(--wpds-dimension-gap-sm)" matches dynamicTokensAST and would be reported with the dynamicToken message, even though --wpds-dimension-gap-sm is entirely static and valid. Meanwhile, staticTokensAST won't catch it because expressions.length > 0.
Opus' suggestion:
A more precise approach could check whether the expression is actually adjacent to (i.e., part of) the --wpds-* token name. For instance, you could check if the TemplateElement's raw value ends with an incomplete token (like --wpds-dimension-gap-) rather than simply containing the prefix. Alternatively, the dynamicTokensAST handler could inspect the raw value to see whether --wpds- appears followed by a complete token name boundary (like ) or whitespace) — if so, it's static and should be validated against the token list, not flagged as dynamic.
At minimum, a test case for this scenario should be added (and probably marked as a known limitation if not addressed):
// Should be valid — the token is static, only another part is dynamic
`${ prefix }: var(--wpds-color-fg-content-neutral)`| code: `const token = 'var(--wpds-nonexistent-token)';`, | ||
| errors: [ | ||
| { | ||
| messageId: 'onlyKnownTokens', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: 'const token = `var(--wpds-nonexistent-token)`;', | ||
| errors: [ | ||
| { | ||
| messageId: 'onlyKnownTokens', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: 'const token = `var(--wpds-dimension-gap-${ size })`;', | ||
| errors: [ | ||
| { | ||
| messageId: 'dynamicToken', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: '<div style={ { gap: `var(--wpds-dimension-gap-${ size })` } } />', | ||
| errors: [ | ||
| { | ||
| messageId: 'dynamicToken', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: `const token = '--wpds-nonexistent-token';`, | ||
| errors: [ | ||
| { | ||
| messageId: 'onlyKnownTokens', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| code: 'const style = `--wpds-dimension-gap-${ size }`;', | ||
| errors: [ | ||
| { | ||
| messageId: 'dynamicToken', | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Should we add a data object to the errors array in these assertions, similarly to what done for other assertions earlier in the same file?
These were already fixed. But yes the point is that those kinds of problems will be caught with these improvements. |
What?
Two improvements to the
no-unknown-ds-tokensESLint rule:styleattributes.--wpds-*token names are dynamically constructed via template literal expressions.Why?
The previous rule only caught invalid tokens written directly in JSX
styleprops. Tokens defined in plain objects or variables (then passed intostyleindirectly) were invisible to the rule. For example:Additionally, dynamically constructed token names like
var(--wpds-dimension-gap-${size})can't be statically verified for validity, and will also prevent the token fallback build plugin from injecting fallbacks.How?
JSXAttribute[name.name="style"]ancestor constraint from the AST selector, so the rule matches--wpds-*in any string literal or template literal.dynamicTokenmessage).Testing Instructions
Try to modify a component file (e.g.
packages/ui/src/stack/stack.tsx) to add invalid tokens, or dynamically generated tokens. They should error now.