-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Theme: Add build plugins to inject design token fallbacks #75589
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 all commits
ec88b59
034c014
f0aaec5
b2083a8
5206004
cb79fc9
0e64b79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| declare const plugin: import('esbuild').Plugin; | ||
| export default plugin; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { readFile } from 'fs/promises'; | ||
| import { addFallbackToVar } from '../postcss-plugins/ds-token-fallbacks.mjs'; | ||
|
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. Non-blocking nit: While
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. The plugins themselves are pretty thin so I'd say there's not much meaningful tests we can add at this level that's worth the trouble. I'd lean towards adding targeted tests once we encounter subtle issues we didn't foresee, or if it ends up we edit these plugin files somewhat frequently (hopefully not!). |
||
|
|
||
| const LOADER_MAP = { | ||
| '.js': 'jsx', | ||
| '.jsx': 'jsx', | ||
| '.ts': 'tsx', | ||
| '.tsx': 'tsx', | ||
| '.mjs': 'jsx', | ||
| '.mts': 'tsx', | ||
| '.cjs': 'jsx', | ||
| '.cts': 'tsx', | ||
| }; | ||
|
|
||
| /** | ||
| * esbuild plugin that injects design-system token fallbacks into JS/TS files. | ||
| * | ||
| * Replaces bare `var(--wpds-*)` references in string literals with | ||
| * `var(--wpds-*, <fallback>)` so components render correctly without | ||
| * a ThemeProvider. | ||
| */ | ||
| const plugin = { | ||
|
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. This was something caught while reviewing this PR with the aid of an AI agent: esbuild The esbuild plugin is first in the plugins array ( The // packages/wp-build/lib/build.mjs:1274-1280
const plugins = [
dsTokenFallbacksJs, // ← wins onLoad for matching files
needsEmotionPlugin && emotionPlugin, // ← skipped for those files
wasmInlinePlugin,
externalizeAllExceptCssPlugin,
...createStyleBundlingPlugins( packageDir ),
].filter( Boolean );Suggestion: Verify whether any files in the codebase use both
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. Good catch. After considering the technical cost of adding support for token replacement in Emotion files (which is not impossible), my current assessment is that it would be better for us to migrate off of Emotion rather than adding more tooling to continue supporting it. Maybe we can add a lint to prevent design token usage in Emotion files. In any case, wanting to use design tokens will hopefully be another good motivation for us to migrate off.
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. Added code comment in 5206004
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.
Added in #75872. This should also address your point about token usage not being checked in Emotion files 😄 |
||
| name: 'ds-token-fallbacks-js', | ||
| setup( build ) { | ||
| build.onLoad( { filter: /\.[mc]?[jt]sx?$/ }, async ( args ) => { | ||
| // Skip node_modules. | ||
| if ( args.path.includes( 'node_modules' ) ) { | ||
| return undefined; | ||
| } | ||
|
Comment on lines
+27
to
+29
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. Performance nit: can we move the
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. Looked into it. esbuild's Given the measured overhead is negligible, seems like the in-callback check is the most pragmatic option here. |
||
|
|
||
| const source = await readFile( args.path, 'utf8' ); | ||
|
|
||
| if ( ! source.includes( '--wpds-' ) ) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const ext = args.path.match( /(\.[^.]+)$/ )?.[ 1 ] || '.js'; | ||
|
|
||
| return { | ||
| contents: addFallbackToVar( source ), | ||
|
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. In the both the esbuild and vite plugins, we're replacing all instances of the token strings, including in code comments. This should be safe and still understandable. Ideally we skip code comments, but it would increase the complexity of this plugin in a way that's probably not worth the trouble. |
||
| loader: LOADER_MAP[ ext ] || 'jsx', | ||
| }; | ||
| } ); | ||
| }, | ||
| }; | ||
|
|
||
| export default plugin; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| declare const plugin: import('postcss').PluginCreator< never >; | ||
| export default plugin; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import { addFallbackToVar } from './ds-token-fallbacks.mjs'; | ||
|
|
||
| const plugin = () => ( { | ||
| postcssPlugin: 'postcss-ds-token-fallbacks', | ||
| /** @param {import('postcss').Declaration} decl */ | ||
| Declaration( decl ) { | ||
| const updated = addFallbackToVar( decl.value ); | ||
| if ( updated !== decl.value ) { | ||
| decl.value = updated; | ||
| } | ||
| }, | ||
| } ); | ||
|
|
||
| plugin.postcss = true; | ||
|
|
||
| export default plugin; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| declare const plugin: () => import('vite').Plugin; | ||
| export default plugin; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { addFallbackToVar } from '../postcss-plugins/ds-token-fallbacks.mjs'; | ||
|
|
||
| /** | ||
| * Vite plugin that injects design-system token fallbacks into JS/TS files. | ||
| * | ||
| * Replaces bare `var(--wpds-*)` references in string literals with | ||
| * `var(--wpds-*, <fallback>)` so components render correctly without | ||
| * a ThemeProvider. | ||
| */ | ||
| const plugin = () => ( { | ||
| name: 'ds-token-fallbacks-js', | ||
| transform( code, id ) { | ||
| if ( ! /\.[mc]?[jt]sx?$/.test( id ) ) { | ||
| return null; | ||
| } | ||
| if ( id.includes( 'node_modules' ) ) { | ||
| return null; | ||
| } | ||
| if ( ! code.includes( '--wpds-' ) ) { | ||
| return null; | ||
| } | ||
| // Sourcemap omitted: replacements are small, inline substitutions | ||
| // that preserve line structure, so the debugging impact is negligible. | ||
| return { code: addFallbackToVar( code ), map: null }; | ||
|
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. Non-blocking nit: Returning This is an acceptable trade-off given the simplicity, but a brief comment explaining the choice would help future maintainers. Alternatively, Opus suggests using `magic-string` and importing `ds-token-fallback.mjs` to generate the sourcemapimport MagicString from 'magic-string';
import { tokenFallbackRegex, getFallback } from '../postcss-plugins/ds-token-fallbacks.mjs';
const plugin = () => ( {
name: 'ds-token-fallbacks-js',
transform( code, id ) {
if ( ! /\.[mc]?[jt]sx?$/.test( id ) ) {
return null;
}
if ( id.includes( 'node_modules' ) ) {
return null;
}
if ( ! code.includes( '--wpds-' ) ) {
return null;
}
const s = new MagicString( code );
const regex = /var\(\s*(--wpds-[\w-]+)\s*\)/g;
let match;
while ( ( match = regex.exec( code ) ) !== null ) {
const tokenName = match[ 1 ];
const fallback = tokenFallbacks[ tokenName ];
if ( fallback !== undefined ) {
s.overwrite(
match.index,
match.index + match[ 0 ].length,
`var(${ tokenName }, ${ fallback })`
);
}
}
if ( ! s.hasChanged() ) {
return null;
}
return {
code: s.toString(),
map: s.generateMap( { hires: true } ),
};
},
} );
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. Added comment in 5206004 Also I think the maps wouldn't degrade in a completely unuseful way, because the line numbers will stay the same (or at least adjacent). |
||
| }, | ||
| } ); | ||
|
|
||
| export default plugin; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,26 @@ import babel from 'esbuild-plugin-babel'; | |
| import { camelCase } from 'change-case'; | ||
| import { NodePackageImporter } from 'sass-embedded'; | ||
|
|
||
| // Optional dependency: @wordpress/theme provides plugins that inject fallback | ||
| // values for design system tokens. Fails gracefully when the package is not | ||
| // installed (it is an optional peerDependency). | ||
| let dsTokenFallbacks; | ||
| let dsTokenFallbacksJs; | ||
| try { | ||
| const { default: postcssPlugin } = await import( | ||
| // eslint-disable-next-line import/no-unresolved | ||
| '@wordpress/theme/postcss-plugins/postcss-ds-token-fallbacks' | ||
| ); | ||
| const { default: esbuildPlugin } = await import( | ||
| // eslint-disable-next-line import/no-unresolved | ||
| '@wordpress/theme/esbuild-plugins/esbuild-ds-token-fallbacks' | ||
| ); | ||
| dsTokenFallbacks = postcssPlugin; | ||
| dsTokenFallbacksJs = esbuildPlugin; | ||
| } catch { | ||
| // @wordpress/theme is optional; skip token fallbacks if not available. | ||
| } | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
|
|
@@ -150,8 +170,9 @@ function compileInlineStyle( { cssModules = false, minify = true } = {} ) { | |
|
|
||
| let moduleExports = null; | ||
|
|
||
| // Transform the code: CSS modules and minification. | ||
| // Transform the code: token fallbacks, CSS modules and minification. | ||
| const plugins = [ | ||
| dsTokenFallbacks, | ||
ciampo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cssModules && | ||
| postcssModules( { | ||
| generateScopedName: '[contenthash]__[local]', | ||
|
|
@@ -1251,6 +1272,11 @@ async function transpilePackage( packageName ) { | |
| }, | ||
| }; | ||
| const plugins = [ | ||
| // Note: dsTokenFallbacksJs and emotionPlugin both use esbuild's onLoad | ||
| // hook, which is non-composable — the first to return contents wins. If a | ||
| // file contains --wpds-* tokens, the Emotion transform will be skipped. | ||
| // Avoid using design tokens in Emotion styles until Emotion is removed. | ||
| dsTokenFallbacksJs, | ||
|
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. Is this a plugin that is needed just for Gutenberg or for third-party plugins too?
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. Third-party plugins would benefit from it too if they want to start using tokens in plugins that still need to support versions of WordPress that don't have |
||
| needsEmotionPlugin && emotionPlugin, | ||
| wasmInlinePlugin, | ||
| externalizeAllExceptCssPlugin, | ||
|
|
@@ -1408,12 +1434,13 @@ async function compileStyles( packageName ) { | |
| embedded: true, | ||
| ...getSassOptions( packageDir ), | ||
| async transform( source ) { | ||
| // Process with autoprefixer for LTR version | ||
| const ltrResult = await postcss( [ | ||
| autoprefixer( { grid: true } ), | ||
| ] ).process( source, { from: undefined } ); | ||
| const ltrResult = await postcss( | ||
| [ | ||
| dsTokenFallbacks, | ||
| autoprefixer( { grid: true } ), | ||
| ].filter( Boolean ) | ||
| ).process( source, { from: undefined } ); | ||
|
|
||
| // Process with rtlcss for RTL version | ||
| const rtlResult = await postcss( [ | ||
| rtlcss(), | ||
| ] ).process( ltrResult.css, { from: undefined } ); | ||
|
|
||
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.
These are official exports so anyone could theoretically integrate them into their build tooling. If necessary, we can even export the data source of the fallbacks so people can make their own injection plugins.