Repository navigation
Theme: Move build and Stylelint plugins to @wordpress/theme-build-tools - #83568
Conversation
🤖 PR meta 🤖🎉 PropsIf 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. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: 0 B Total Size: 9.05 MB
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. Regression: updating a specific option will only change its value and will not corrupt others in
|
| try { | ||
| const { default: esbuildPlugin } = | ||
| await import( '@wordpress/theme/esbuild-plugins/esbuild-ds-token-fallbacks' ); | ||
| await import( '@wordpress/theme-build-tools/esbuild-plugins/esbuild-ds-token-fallbacks' ); |
There was a problem hiding this comment.
I know it's a precedent to this PR but are we concerned about wp-build having package specific build tooling?
There was a problem hiding this comment.
Here we don't have specific tooling for building the theme package, but we make wp-build apply a certain build step on everything it builds. Namely transforming all var(--wpds-foo) CSS variable references so that they contain a fallback.
That's a fairly normal build step that uses an external tool. Like the Emotion plugin, various CSS minifiers and postprocessors. And it's WP specific, like many other things: externalizing specific @wordpress/* packages, working with the routing framework, reading specific package.json fields...
These are reasons why I wouldn't be concerned much.
c6e175c to
95a7547
Compare
| }, | ||
| "peerDependencies": { | ||
| "@wordpress/theme": ">=0.8.0 <3.0.0" | ||
| "@wordpress/theme-build-tools": "^1.0.0-prerelease" |
There was a problem hiding this comment.
CI checks are failing because this package (and version) are not publicized yet on npm, maybe we could stage the new package's publication (or make the audit validate the local candidate before relying on this peer) ?
There was a problem hiding this comment.
The audit already supports validating the local candidate when the dependency is declared as file:.... But for peerDependencies the file:... specifier is not valid, there needs to be a version range. But a real version range implies a NPM download which doesn't exist yet... a chicken-egg situation.
A good solution is to land theme-build-tools as a normal dependency first and only after publishing to switch it to peer.
And by the way, maybe the theme build shouldn't be optional/peer at all? The wp-build uses the fallback plugin directly. And the fallbacks should be applied for all users of wp-build, it shouldn't depend on whether the consumer semi-randomly installs theme-build-tools in the same project.
For theme the peer dependency made sense, but for theme-build-tools it probably doesn't, as there is no other user of the build tools in the repo.
There was a problem hiding this comment.
That direct-dependency organisation sounds like a good benefit coming out of this.
There was a problem hiding this comment.
It's the same chicken-and-egg situation with npm OIDC where you can set up OIDC for a package that is already published to npm, which means you can't set up OIDC for a fresh package until you publish it.
So, the hack here is to manually publish the first bare-minimum version and then let the tools do the job.
There was a problem hiding this comment.
A good solution is to land
theme-build-toolsas a normal dependency first and only after publishing to switch it to peer.
Implemented in 0e35967, and I think it solves the CI issue, although it now enforces the theme version shipped with this package instead of one defined by the consumer. That seems fine. I can follow up to relax it again after package releases in a couple of weeks.
And by the way, maybe the theme build shouldn't be optional/peer at all? The
wp-builduses the fallback plugin directly. And the fallbacks should be applied for all users ofwp-build, it shouldn't depend on whether the consumer semi-randomly installstheme-build-toolsin the same project.
There's cons/pros; optional/peer allows flexible Theme package versioning (and thus different WPDS tokens), but I think we should just take a stance that the linter and theme package should be updated together (i.e. they should match).
| "@babel/types": "^7.29.7", | ||
| "@jridgewell/remapping": "^2.3.5", | ||
| "@jridgewell/trace-mapping": "^0.3.31", | ||
| "@wordpress/theme": "file:../theme", |
There was a problem hiding this comment.
This is the part I'm concerned about. The theme plugins are specifically coupled to the token data of any given theme package version. In the old setup, we could guarantee that the plugins match the theme package version that the consumer has installed. With this separate package, the theme version is now decoupled and kind of opaque. (I checked that before publishing Lerna replaces the file: expression with a normal registry version range, e.g. ^3.0.0.)
I'm afraid this may cause version mismatch confusion.
There was a problem hiding this comment.
Yeah, this is the main concern for me too, we may risk introducing even more compat / version syncing troubles than the ones we're trying to solve 🤔
There was a problem hiding this comment.
There should be a clear separation between
- the token and theme data themselves
- a tool that uses and applies them
If we have that clear separation (I think we do), then there shouldn't be a big concern. It's just like with any other build tool. For example, we can have an older version of browserslist, and still have the latest fresh database with browser data from caniuse-lite.
There was a problem hiding this comment.
If I'm not mistaken, the new setup means that package managers can install a nested copy of wp-theme here, which could be different from the wp-theme version that the consumer thinks they are using. This is a problem because the consumer needs to be using wp-theme validation that matches their target WP version, not the "latest fresh" tokens.
There was a problem hiding this comment.
the new setup means that package managers can install a nested copy of
wp-theme
Oh yes, this looks like a bug, related to the dependency-audit discussion in the other thread. wp-theme should be a peer dependency of wp-theme-build-tools, because wp-theme has other usages in a project. And wp-theme-build-tools can be a direct dependency (not peer) of wp-build because it's just the tooling code that wp-build uses directly. And wp-theme-build-tools is supposed to get its token data from the installed peer. And fail gracefully when the peer is not found.
There was a problem hiding this comment.
That's cool, so outcome is that @wordpress/build and @wordpress/theme-build-tools don't need to stay in sync with @wordpress/theme version installed; but once they bump @wordpress/theme version and there are changes to tokens, consumers gets to update their CSS files as the linter will pick the changes up.
There was a problem hiding this comment.
Looking at the latest commit and its message, it looks like the plan is to use a direct dependency temporarily, then restore the optional peer after publication.
Let's make sure we keep track of that follow-up, and plan to verify that validation uses the app's selected theme version when another version is installed elsewhere in the dependency tree.
95a7547 to
0549390
Compare
ff753be to
bacfda3
Compare
|
@ciampo yep, I wrote this comment earlier but looks like I didn't post it. @jsnajdr and @manzoorwanijk particularly since you noted before in that convo, how does this look now regarding versioning? |
jsnajdr
left a comment
There was a problem hiding this comment.
This looks good now 👍
The theme-tokens module could be a bit simplified, and the imports better explained, but these are non-blockers.
0e35967 to
f42e71c
Compare
f42e71c to
bb5460d
Compare
Move the design token fallback build plugins (PostCSS, Lightning CSS, esbuild, Vite) and the Stylelint plugins from @wordpress/theme to a new @wordpress/theme-build-tools package, keeping the same subpath exports. The old @wordpress/theme/*-plugins/* exports are removed. Update @wordpress/build, @wordpress/stylelint-config, Storybook and repo tooling to load the plugins from the new package.
Make @wordpress/theme an optional peer dependency of @wordpress/theme-build-tools, so the plugins use the token data of the theme version installed in the project, and do nothing when it is missing. Make @wordpress/theme-build-tools a regular dependency of @wordpress/build, which now imports the plugins directly.
The design-token-fallbacks.js export the plugins read first ships in the next major release of @wordpress/theme. Drop the dependency audit ignore rule that covered resolving the peer to 2.2.0.
…xport Derive the token list and fallback map from the tokens named export of @wordpress/theme/design-tokens.js, added in #76604, instead of a separate design-token-fallbacks.js export. With an older @wordpress/theme, the Stylelint rule uses the default token-name list and the fallback plugins do nothing. Also require Node.js >=22.13.0 and stop listing Markdown files in `files`, matching the other packages.
Remove a @wordpress/build changelog entry the rebase duplicated under the released 0.25.0 heading, refresh the lockfile, and name the renamed tsconfig.prebuilt.json in the changelog workflow comment.
Carry over the bump from #84544, which updated it in @wordpress/theme before the dependency moved to this package.
Top-level await in the token loader made Node reject require() of every plugin with ERR_REQUIRE_ASYNC_MODULE, so a CommonJS config could not just swap @wordpress/theme for @wordpress/theme-build-tools. Load the optional @wordpress/theme token data synchronously with createRequire instead. Test that the plugins load with require(), and hide @wordpress/theme from both import and require() in the test without it.
Make @wordpress/theme a regular dependency for now, instead of an optional peer. A peer range on a @wordpress/theme version that is not on npm yet fails the published-dependency audit, while a file: dependency is checked against the local package. It also means the first release always injects fallbacks, without consumers installing @wordpress/theme. Switch back to an optional peer once the @wordpress/theme release with the tokens export is published.
The rebase over the 2026-10-20 release merged this branch's entries into the newly released @wordpress/theme 3.0.0, @wordpress/build 0.26.0 and @wordpress/stylelint-config 27.0.0 sections.
@wordpress/theme is now a dependency at this branch's version, which always has the tokens export, so read the token names from it instead of falling back to the default export of older versions.
Return undefined for any error when loading @wordpress/theme token data, instead of only for a missing package or subpath, as suggested in review.
bb5460d to
eebfc2c
Compare
|
Follow-up to continue support different versions of the installed The ranges would need to be updated manually, which is why I'm also looking at linting peer ranges: |
Depends on #76604
Previously:
What?
Moves the design token build plugins (PostCSS, Lightning CSS, esbuild, Vite) and the Stylelint rules from
@wordpress/themeto a new package,@wordpress/theme-build-tools. The subpaths after the package name stay the same.The old
@wordpress/theme/*-plugins/*exports are removed, so consumers have to migrate when updating.Update
@wordpress/build,@wordpress/stylelint-config, Storybook and repo tooling to load the plugins from the new package.Why?
The build tools only run at build time, so their peers don't belong on a runtime package. After this change,
@wordpress/themeonly has React peers.1. Avoid consumer bundles ballooning due to duplicate versions of
@wordpress/themeor@wordpress/uidue to mismatched@types/nodeversions.Package manager
pnpmcreates a separate package instance for each distinct set of resolved peers, including transitive ones such asvite→@types/node. In a workspace where two@types/nodeversions coexist,@wordpress/theme, and@wordpress/uithrough it, resolve twice, so bundles ship duplicate copies.dedupePeerDependentscan't merge them, because the peer sets really differ.This happens with
pnpm(and anything else that copies packages per peer set). npm's hoistednode_modulesdoesn't split packages this way.With the old peers: when a bundle's dependencies reach
@wordpress/themethrough different vite copies, the bundle gets two copies of theme. It also gets two copies of@wordpress/ui, because ui depends on theme. Each copy lives in its own folder, and the bundler includes both.Without them:
@types/nodeskew only splitsvite. That's a build tool that never goes into a bundle, so nothing extra ships to the browser.2. Make maintaining build tool peer dependencies å separate concern from runtime package
Suggested in #80952 (comment), although the tooling split to
themeanduididn't end up happening:How?
packages/theme-build-tools@wordpress/theme's public exports. This adds one new export:@wordpress/theme/design-token-fallbacks.js(code). The new public export also makes it easier for consumers to create their own plugins. Did not exportdesign-tokensfile (code).Testing Instructions
npm install && npm run build.npm run test:unit:vitest -- packages/theme-build-tools packages/theme packages/wp-build packages/stylelint-config.npm run typecheck && npm run lint:css.var(--wpds-color-foreground-content-neutral, #1e1e1e).npm pack --dry-runinpackages/themeand check that no*-pluginsdirectories are included.CI:
lint:published-depsfails until@wordpress/theme-build-toolsis published, because@wordpress/build's peer range has to be fetched from npm.Use of AI Tools
claude