Add theme package
#72305
Merged
Add theme package
#72305
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsnajdr
reviewed
Oct 14, 2025
Member
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
One thing that is missing in the PR are the build/css/design-tokens.css and build/js/design-tokens.js that are referenced by the exports field in package.json.
What if we moved them to a directory that's not in gitignore? And updated the build scripts and package.json accordingly. Having the files in build causes a lot of inconvenience when you want to clean the package folder: unlike any other package, you can't just delete the build folder.
| DEFAULT_SEED_COLORS, | ||
| buildBgRamp, | ||
| buildAccentRamp, | ||
| } from '../../src/color-ramps/index'; |
Member
There was a problem hiding this comment.
I always though that the .ts extensions which were stripped here are required by Node.js. When running a script and resolving these imports with its native resolver, file extensions are mandatory.
I was suprised to see that npm run build:tokens works with the removed extensions. And then figured out that we first compile this script with esbuild and only then we run it with Node. The bin/build-tokens.js script does that.
Maybe we could simplify this in the future and avoid the build step?
Member
Author
There was a problem hiding this comment.
Node < 22.6 cannot run TS files, and apparently it's a whole ordeal to update the node version on the .org build server. Some approaches I considered:
- Use a different node version just for this package. Not great for devex, but also, this package also contains normal React stuff that I assume will need to be built on the build server.
- Use
ts-nodeortsxinstead of node. This works for thegenerate-primitive-tokensscript, but not the Terrazzo build, since we need to use thetzCLI tool. - Convert all the files back to plain JS. This is a waste of time, given we'll eventually want TS anyway.
So until this entire project is ready for Node 22.6+, I thought it would be simplest to have this temporary intermediate build script that builds the TS files first. The build script can eventually be removed, and we can go back to a simple node scripts/generate-primitive-tokens/index.ts && tz build --config terrazzo.config.ts.
Member
There was a problem hiding this comment.
We'll have do to a make.wordpress.org/systems request sooner or later to update Node to the latest 22.x version. The reason we can give is that Gutenberg starts to use modern tools like esbuild and TypeScript and that the latest Node has some crucial new features:
- ability to run
.tsfiles directly without any transpilation. Some of our build scripts are already TS-enabled - new APIs like
findPackageJSONthat are very useful for our esbuild plugins
I don't have permission to post to make.wordpress.org/systems, only to /core. @aduth or @tyxla can you help with that? I can provide the whole text of the post to be posted 🙂
For now, the intermediate esbuild step is OK, although not particularly elegant. The alternative is to strip types from the build scripts and convert them to plain .js. But one day we'd be adding the types back.
Member
Author
There was a problem hiding this comment.
Jarda and I discussed this today, and agreed that we should wait until October 28 when Node 24 enters Active LTS, and then request an update straight to 24.
Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
|
Size Change: +28.4 kB (+1.3%) Total Size: 2.21 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in ddfa586. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18639889295
|
# Conflicts: # bin/packages/build-v2.mjs
Due to "moduleResolution": "bundler" tsconfig in this package
# Conflicts: # bin/packages/v2-packages.js
mirka
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
as code owners
Member
Author
|
@WordPress/gutenberg-components Ok I think this is ready for review 😮💨 |
|
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. |
mirka
commented
Oct 20, 2025
| const cssModuleEntries = await glob( | ||
| normalizePath( path.join( packageDir, 'src/**/*.module.css' ) ), | ||
| { ignore: IGNORE_PATTERNS } | ||
| ); |
Member
Author
There was a problem hiding this comment.
I set up a separate process for CSS modules. It's currently "opinionatedly modern", in that we don't support RTL transformation or Sass.
Comment on lines
+945
to
+950
| // Generate combined stylesheet from all CSS modules | ||
| if ( cssResults.length > 0 ) { | ||
| const combinedCss = cssResults.join( '\n' ); | ||
| await mkdir( buildStyleDir, { recursive: true } ); | ||
| await writeFile( path.join( buildStyleDir, 'style.css' ), combinedCss ); | ||
| } |
Member
Author
There was a problem hiding this comment.
This "combine step" isn't necessary for this theme package that only has one stylesheet, but it will be necessary for the UI components package which will have separate stylesheets for every component.
The idea is that most consumers can just import the single combined stylesheet for convenience, while size-sensitive consumers can opt to import the separate component-specific stylesheets if they want.
Member
|
We discussed in which folders we should put various files generated by build scripts. After a closer look I see there are three types of artifacts and three stages of build. First step: the DTCG tokens in These DTCG tokens are not currently exported from the Second step: generate various files from the DTCG info, using the Terrazzo tool:
In this step I propose two changes:
Third step: Transpile all source files with the standard tooling, producing The |
Member
Author
My preference would be punt this to follow up, though. Is that fine?
The current behavior is this:
So the destinations are |
jsnajdr
approved these changes
Oct 20, 2025
Member
jsnajdr
left a comment
jsnajdr
left a comment
There was a problem hiding this comment.
My preference would be punt this to follow up, though. Is that fine?
Yes, this looks like a perfectly good initial version of the package. My proposals are mostly fine-tuning, certainly not blockers.
One language nit: maybe src/prebuilt is a better name than src/prebuild 🙂 The files have been prebuilt and committed.
This was referenced Oct 22, 2025
Closed
This was referenced Oct 31, 2025
Member
|
Some updates on the "punted for follow-up" actions:
Still blocked by #72593
Added in #72747
Created tracking issue at #72891
The changes in #72890 change
Stylesheet inlining has been handled through a combination of #72673 and #72805 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #71196
What?
Adds a
@wordpress/themepackage to handle theming concerns of the WordPress design system.Punted to follow-up
ThemeProvider. I omitted them for now (d377c0c), because we cannot do these snapshot tests effectively without upgrading the repo to Jest 30. (jsdom < 26 will strip out CSS variables from the output.)design-tokens.jsanddesign-tokens.tscan probably be consolidated into a single JS file.styleattribute.Why?
This package of design tokens will be the foundation to our design system, and is a requirement for our new UI components package.
Code Review Instructions
Most of the core code has been reviewed before. I've highlighted any notable changes in the inline comments.
Structural decisions to discuss:
src/prebuildfolder. The only exceptions aretokens/color.jsonanddocs.scriptsfolder is also used at runtime. I tried renaming it tobinto make it clearer that they're only used at compile time.Testing Instructions
See the output of the builds and confirm that CSS modules are being processed as expected. You can also try importing the
ThemeProvidercomponent into the app or a Storybook story to see how it handles CSS modules. (Note that the stylesheet will need to be imported separately.)