Stylelint: no WPDS token fallback values for WP Build routes - #50502
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
ad86d20 to
798102e
Compare
798102e to
c9137da
Compare
| }, | ||
| ], | ||
| }, | ||
| overrides: [ |
There was a problem hiding this comment.
I'd really rather we don't have a big list of permanent overrides in this global configuration file, that people will have to know to maintain. Are these meant to be temporary? If not, please move these into stylelint config files in each project.
There was a problem hiding this comment.
I agree and they're indeed meant to be temporary and going away as soon as everything can be linted with the same rule across the repo. It's just easier to migrate in smaller chunks, and this was most pragmatic temporary solution.
Hopefully no longer than just this week!
There was a problem hiding this comment.
Aight, got pretty far!
Among files with --wpds-* the linter + build time fallbacks now cover:
| Covered | Total | % | |
|---|---|---|---|
| Files | 147 | 173 | 85.0% |
| Packages | 16 | 21 | 76.2% |
Not on the rule: charts, components, base-styles, social-previews, plugins/jetpack
I might flip the rule the other way around; ignore those and apply everywhere else.
Those others are pretty complicated:
- charts and social-previews use LightningCSS, not PostCSS
- Jetpack plugin still relies on old Calypso colour schemes a little too much
- components and base-styles are just unique cases otherwise
I will continue after short vacation. :-)
There was a problem hiding this comment.
I might flip the rule the other way around; ignore those and apply everywhere else.
| display: inline-flex; | ||
| align-items: center; | ||
| gap: var(--wpds-dimension-gap-xs, 4px); | ||
| gap: var(--wpds-dimension-gap-xs); |
There was a problem hiding this comment.
It seems the plugin didn't add a fallback for this one for some reason.
There was a problem hiding this comment.
🤔 Just checked it's a real valid token.
https://wordpress.github.io/gutenberg/?path=/docs/design-system-tokens-introduction--docs#dimension
| */ | ||
| padding-block-end: 24px; | ||
| padding-inline: var(--wpds-dimension-padding-2xl, 24px); | ||
| padding-inline: var(--wpds-dimension-padding-2xl); |
There was a problem hiding this comment.
It seems the plugin didn't add a fallback for this one for some reason.










Follow-up to #50110 which added linting for WPDS tokens but didn't enforce the "no fallbacks" rule yet.
Proposed changes
Enforces "no fallbacks" rule to paths which are built using
wp-Buildtooling: build pipeline inwp-buildalready adds fallbacks at build-time, so this is a safe change.Manually maintaining fallbacks is error-prone and requires more work during
@wordpress/themeupdates.List of paths is manually maintained list since it's meant to be temporary. I'm working through changes across codebase to add fallbacks in Webpack build bundles (#50108). Once that's finished, we can start linting the rest of the stylesheets as well.
On wp-build admin pages, WPDS tokens are always loaded and manual fallbacks are ignored — the browser always uses the token value from the theme.
However, there were some mismatches, and it's worth documenting just in case.
See list by severity
High — wrong token semantics or large visual delta
--wpds-color-stroke-interactive-neutral-strong#1d35b4(admin blue)#6e6e6e(neutral gray)videopress/routes/overview/style.scss--wpds-color-stroke-surface-brand-strongor--wpds-color-stroke-interactive-brand.--wpds-border-radius-md8px4pxpremium-analytics/.../post-summary-card.module.scss8pxis--wpds-border-radius-lg, notmd. Likely copy-paste error.--wpds-border-radius-sm6px2px.image)Medium — noticeable color differences
--wpds-color-stroke-surface-neutral#e0e0e0,#f0f0f1#dbdbdb--wpds-color-foreground-content-neutral-weak#50575e,#3c434a,#757575#707070The newsletter dashboard even has TODOs about wanting a “muted text” tone — the removed values were WordPress admin grays (
#50575e≈$gray-50,#3c434a≈$gray-60), not WPDS#707070. With tokens loaded, muted text was already#707070.Low — minor or structural differences
--wpds-typography-font-family-headingforms/routes/response/style.scss— minor font fallback difference only when tokens missing--wpds-color-stroke-focus#1d35b4#3858e9(runtime) /var(--wp-admin-theme-color, #3858e9)(PostCSS fallback)videopress/routes/overview/style.scss— different blue; now theme-aware--wpds-border-width-focus2pxvar(--wp-admin-border-width-focus, 2px)2pxRelated product discussion/links
Does this pull request change what data or activity we track or use?
Testing instructions
Smoke test affected routes:
No differences in bundles, apart from corrected fallback values (they're now canonical rather than incorrect).