Repository navigation
ESLint: Broaden no-unknown-ds-tokens to all strings and catch dynamic construction
#75905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
4b567c5
30f0cc1
de4a043
5773294
1d70dd6
8f0605b
2df9954
ddd7fd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| /* eslint-disable @wordpress/no-unknown-ds-tokens */ | ||
| import { RuleTester } from 'eslint'; | ||
| import rule from '../no-unknown-ds-tokens'; | ||
|
|
||
|
|
@@ -27,6 +28,9 @@ ruleTester.run( 'no-unknown-ds-tokens', rule, { | |
| { | ||
| code: '<div style={ { color: `var(--wpds-color-fg-content-neutral)` } } />', | ||
| }, | ||
| { | ||
| code: `const token = 'var(--wpds-color-fg-content-neutral)';`, | ||
| }, | ||
| ], | ||
| invalid: [ | ||
| { | ||
|
|
@@ -56,5 +60,38 @@ ruleTester.run( 'no-unknown-ds-tokens', rule, { | |
| }, | ||
| ], | ||
| }, | ||
| { | ||
| 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: [ | ||
|
Comment on lines
+99
to
+109
|
||
| { | ||
| messageId: 'dynamicToken', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| } ); | ||
| /* eslint-enable @wordpress/no-unknown-ds-tokens */ | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,33 +42,40 @@ module.exports = /** @type {import('eslint').Rule.RuleModule} */ ( { | |||||||||||||||||||||||||
| meta: { | ||||||||||||||||||||||||||
| type: 'problem', | ||||||||||||||||||||||||||
| docs: { | ||||||||||||||||||||||||||
| description: 'Prevent use of non-existing --wpds-* variables', | ||||||||||||||||||||||||||
| description: 'Prevent use of non-existing --wpds-* variables', // eslint-disable-line @wordpress/no-unknown-ds-tokens | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| schema: [], | ||||||||||||||||||||||||||
| messages: { | ||||||||||||||||||||||||||
| onlyKnownTokens: | ||||||||||||||||||||||||||
| 'The following CSS variables are not valid Design System tokens: {{ tokenNames }}', | ||||||||||||||||||||||||||
| dynamicToken: | ||||||||||||||||||||||||||
| 'Design System tokens must not be dynamically constructed, as they cannot be statically verified for correctness or processed automatically to inject fallbacks.', | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| create( context ) { | ||||||||||||||||||||||||||
| const disallowedTokensAST = `JSXAttribute[name.name="style"] :matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral TemplateElement[value.raw=${ wpdsTokensRegex }])`; | ||||||||||||||||||||||||||
| const dynamicTokensAST = `TemplateLiteral[expressions.length>0] TemplateElement[value.raw=${ wpdsTokensRegex }]`; | ||||||||||||||||||||||||||
| const staticTokensAST = `:matches(Literal[value=${ wpdsTokensRegex }], TemplateLiteral[expressions.length=0] TemplateElement[value.raw=${ wpdsTokensRegex }])`; | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 const style = `color: ${color}; gap: var(--wpds-dimension-gap-sm)`;The template element Opus' suggestion: A more precise approach could check whether the expression is actually adjacent to (i.e., part of) the // Should be valid — the token is static, only another part is dynamic
`${ prefix }: var(--wpds-color-fg-content-neutral)`
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I guess same as #75905 (comment))
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| [ dynamicTokensAST ]( node ) { | ||||||||||||||||||||||||||
| context.report( { | ||||||||||||||||||||||||||
| node: node.parent, | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| 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, |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as #75905 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.