Theme: Rename typography tokens to use "typography" prefix#76912
Theme: Rename typography tokens to use "typography" prefix#76912
Conversation
|
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. |
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
There was a problem hiding this comment.
Should we add tests also for ds-token-fallbacks.mjs , since that's the actual file used by plugins?
packages/theme/CHANGELOG.md
Outdated
| ## Unreleased | ||
|
|
||
| ### Enhancement | ||
| ### Breaking changes |
There was a problem hiding this comment.
A few reflections on these breaking changes: if consumers have the plugin installed, it will actually throw and consumers will be forced to update values.
But if they don't have linters / build plugins installed, those failures will be silent — so maybe we should insist more on recommending consumers of the theme package to have the build/linters installed too?
Finally, since @wordpress/theme styles are "global" (ie not bundled), what would happen for plugin authors that need to support multiple WordPress versions? Would they need to write something like color: var( --new-token, var( --old-token, fallback_value ) ) ?
There was a problem hiding this comment.
But if they don't have linters / build plugins installed, those failures will be silent — so maybe we should insist more on recommending consumers of the theme package to have the build/linters installed too?
What would that look like? Would a recommendation in README.md suffice? Or are you thinking some install-time messaging, or maybe even something more extreme where @wordpress/theme doesn't resolve except when being built through @wordpress/build ?
Finally, since
@wordpress/themestyles are "global" (ie not bundled), what would happen for plugin authors that need to support multiple WordPress versions? Would they need to write something likecolor: var( --new-token, var( --old-token, fallback_value ) )?
I think this could be somewhat related to your first question, where if we could assume developers were using the build tools, they'd have that fallback value for free and the build-time validation to be using the correct tokens.
The other part of this being an assumption that this is a private API that we don't expect developers to be using yet and that's why we're making these changes now, because we can't make them later.
There was a problem hiding this comment.
It's probably ok for now, given that we're treating this as private / alpha, but we should have clear answers to these questions for when we make the APIs public.
We can probably start with a README recommendation, and keep eyes out for eventually adding stricter checks?
We should also test those fallback scenarios out, so that we can potentially even add a section specific for supporting multiple versions of the package, if necessary?
There was a problem hiding this comment.
But if they don't have linters / build plugins installed, those failures will be silent — so maybe we should insist more on recommending consumers of the theme package to have the build/linters installed too?
Linters aren't required IMO, but I do think it's important to fail loudly in the build when a fallback can't be injected (I have an issue for that at #76842).
And I was going to update the readmes for all design token consumer packages on the recommended/required CSS setup, since they're currently outdated. There's actually like a "minimal mode" and a "full theming mode" for token usage right now, which have different setup requirements based on the environment you're building for. And in a subset of those setups, using wp-build should be required.
what would happen for plugin authors that need to support multiple WordPress versions?
Generally agree with @aduth in that all consumers building for WP should be using the wp-build tooling, and in that case the fallbacks will give you an automatic layer of cross-version safety in most cases. Things will be a bit more complicated when we start to have things like dark mode sections in WP, where a lack of dynamic theming in a plugin's design tokens can result in elements being invisible.
There was a problem hiding this comment.
I updated the @wordpress/theme package README in ac42c18 to reflect how we might expect consumers to use the design tokens, including build toolings and linters. Does it read alright to you both?
| throw new Error( | ||
| `Unknown design token: ${ tokenName }. ` + | ||
| 'This token is not in the design system. ' + | ||
| 'If this token was recently renamed, update all references to use the new name.' | ||
| ); |
There was a problem hiding this comment.
The generic TypeScript version in add-fallback-to-var.ts should remain lenient
This quote is from the original action plan, although it looks like add-fallback-to-var.ts also throws on unknown tokens — was that deliberate or an AI agent slip-up?
Another consequence of making both functions strict is that some false-positives in non-CSS contexts are caught, too (JS test strings, JSDoc examples) such as the --wpds-border-fallback CI error.
Maybe the best solution is to keep the strict throw, but make it conditional? ie. only throw for PostCSS (actual CSS files) but stay lenient in the esbuild/Vite JS transforms. This way, stale token references in CSS are caught at build time, but harmless string literals in JS test files don't break the build.
Alternatively, we could narrow the regex scope to skip tests and JS comments?
There was a problem hiding this comment.
The generic TypeScript version in add-fallback-to-var.ts should remain lenient
This quote is from the original action plan, although it looks like
add-fallback-to-var.tsalso throws on unknown tokens — was that deliberate or an AI agent slip-up?
The original build of the plan included strict as an option for throwing errors, and I subsequently prompted it to remove the option and make strict the default behavior, so that might explain the disparity of the plan.
Although as you also point out, this has some cascading side-effects. I'll take a look to see if it's easy enough to clean those up / rein in the pattern matching, or if we should reincorporate some leniency.
There was a problem hiding this comment.
6d556bf updates the code to use valid token names, which feels better in context as-is. It doesn't address the general problem that the build tool could flag some invalid tokens that we probably shouldn't care about (e.g. code comments), but I think it's generally better to start strict and we can relax it if real-world usage surfaces cases where it's not reasonable to change to a valid token like we've done here.
|
Size Change: -2.06 kB (-0.03%) Total Size: 7.74 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in ea83373. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23954200435
|
packages/theme/README.md
Outdated
|
|
||
| Design tokens are delivered as CSS custom properties (e.g. `var(--wpds-color-fg-content-neutral)`). To use them, a stylesheet defining the token values must be loaded on the page. | ||
|
|
||
| Within WordPress, this stylesheet is loaded automatically. Outside of WordPress, you can import the prebuilt stylesheet directly from `@wordpress/theme/src/prebuilt/css/design-tokens.css`. The [Theme Provider](#theme-provider) component can be used on top of either approach to customize token values like colors and density for a subtree of the DOM. |
There was a problem hiding this comment.
subtree of the DOM
Nitty, but part of the point of ThemeProvider is that it can pass theme settings down React subtrees, not just DOM. So maybe a more generic wording like "a subsection of your app" would be fine.
There was a problem hiding this comment.
subtree of the DOM
Nitty, but part of the point of
ThemeProvideris that it can pass theme settings down React subtrees, not just DOM. So maybe a more generic wording like "a subsection of your app" would be fine.
Yeah, good point. 👍 I rephrased this in 72e68f8.
72e68f8 to
ea83373
Compare
Otherwise these are flagged by build-time validation on valid token names. In these scenarios, real valid token names should be used anyways.
Encourage usage of build tooling and linters
ea83373 to
0db5477
Compare
What?
Closes #76838
typography-prefix, removingfont-from line-height tokens.Why?
typography.json➡️wpds-typography), consistent with other tokensTesting Instructions
Verify there are no more occurrences of
--wpds-font-*tokens in the codebase.Verify that
npm run buildandnpm run lint:jsboth pass. These would be expected to fail if there were invalid tokens, via PostCSS transform build-time errors and valid token linters.Use of AI Tools
Anthropic Claude Opus 4.6 in Plan Mode was used to plan and execute the build.
Rename plan
Rename
wpds-fonttowpds-typographyImpact Analysis
The rename affects ~150 occurrences across ~24 files in 6 packages. The generated prebuilt files (CSS, JS, TS, docs) will be regenerated automatically by the build.
This is a breaking change for
@wordpress/theme. The package is used externally, so the CHANGELOG must document the rename with a migration table.The TypeScript types (
FontFamily,FontSize,FontWeight,LineHeight) are explicitly named interrazzo.config.tsand do not need renaming -- they remain semantically correct.Naming Strategy
Sub-groups that correspond to CSS
font-*properties retain thefont-prefix.line-heightis not a font property and drops the prefix:--wpds-font-family-*--wpds-typography-font-family-*--wpds-font-size-*--wpds-typography-font-size-*--wpds-font-weight-*--wpds-typography-font-weight-*--wpds-font-line-height-*--wpds-typography-line-height-*This means 4 distinct find-and-replace patterns in consumer files (not a single global replace).
Changes
1. Strict fallback validation (prerequisite)
Modify
[packages/theme/src/postcss-plugins/ds-token-fallbacks.mjs](packages/theme/src/postcss-plugins/ds-token-fallbacks.mjs)to throw an error when avar(--wpds-*)reference is encountered that is not in the generated fallback map. Currently the function silently skips unknown tokens (line 31:if (fallback === undefined) { return match; }).The change: replace the early return with a thrown
Errorincluding the unrecognized token name. This ensures any stale--wpds-font-*references cause a build failure with a clear message, both within Gutenberg and for external consumers using the build plugins.The generic TypeScript version in
[add-fallback-to-var.ts](packages/theme/src/postcss-plugins/add-fallback-to-var.ts)should remain lenient (it takes arbitrary fallback maps and is used in tests with partial maps).Add a test case to
[add-fallback-to-var.test.ts](packages/theme/src/postcss-plugins/test/add-fallback-to-var.test.ts)verifying the error throw behavior by importing fromds-token-fallbacks.mjsor by adding a dedicated test for the strict behavior.2. Token source file
Restructure
[packages/theme/tokens/typography.json](packages/theme/tokens/typography.json):"wpds-font"to"wpds-typography""family"to"font-family""size"to"font-size""weight"to"font-weight""line-height"as-is3. Terrazzo build config
Update
[packages/theme/terrazzo.config.ts](packages/theme/terrazzo.config.ts):token.id.startsWith('wpds-font.weight.')->'wpds-typography.font-weight.'wpds-font.family->wpds-typography.font-familywpds-font.size->wpds-typography.font-sizewpds-font.weight->wpds-typography.font-weightwpds-font.line-height->wpds-typography.line-height4. Regenerate prebuilt files
Run
npm run --workspace @wordpress/theme build:tokensto regenerate:packages/theme/src/prebuilt/css/design-tokens.csspackages/theme/src/prebuilt/js/design-tokens.mjspackages/theme/src/prebuilt/js/design-token-fallbacks.mjspackages/theme/src/prebuilt/ts/token-types.ts(type names stay the same)packages/theme/docs/tokens.md5. Consumer files (4 replacement patterns)
Apply these replacements across all consumer CSS/SCSS/TSX files:
--wpds-font-family->--wpds-typography-font-family--wpds-font-size->--wpds-typography-font-size--wpds-font-weight->--wpds-typography-font-weight--wpds-font-line-height->--wpds-typography-line-height**@wordpress/ui** (12 style files + 2 story files)**@wordpress/admin-ui(2 files)**@wordpress/dataviews(1 file)Storybook (1 file)
6. Test files
[packages/theme/src/postcss-plugins/test/add-fallback-to-var.test.ts](packages/theme/src/postcss-plugins/test/add-fallback-to-var.test.ts): Update 6 occurrences in mock fallback data and assertions[packages/eslint-plugin/rules/__tests__/no-setting-ds-tokens.js](packages/eslint-plugin/rules/__tests__/no-setting-ds-tokens.js): Update 2 occurrences in test code strings7. CHANGELOG
Add a breaking change entry to
[packages/theme/CHANGELOG.md](packages/theme/CHANGELOG.md)under## Unreleasedwith the full migration table.8. Verification
npm run --workspace @wordpress/theme buildto confirm token build succeedsnpm run test:unit packages/themenpm run test:unit packages/eslint-pluginwpds-fontreferences to confirm completeness--wpds-font-references survive in code processed by the PostCSS/esbuild/Vite plugins, the build will fail with a clear error