Theme: Promote ThemeProvider to stable API#78958
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. |
|
Size Change: -1.24 kB (-0.02%) Total Size: 7.5 MB 📦 View Changed
|
|
Flaky tests detected in b43c2be. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26975139006
|
ciampo
left a comment
There was a problem hiding this comment.
I guess this is unrelated from the the package README that mentions This package is still experimental. "Experimental" means this is an early implementation subject to drastic and breaking changes, right?
| export { privateApis } from './private-apis'; | ||
| export type * from './prebuilt/ts/token-types'; | ||
| export { type ThemeProvider } from './theme-provider'; | ||
| export { ThemeProvider } from './theme-provider'; |
There was a problem hiding this comment.
Now that it;s a public API, we should probably add JSDocs to the ThemeProvider component
There was a problem hiding this comment.
Now that it;s a public API, we should probably add JSDocs to the
ThemeProvidercomponent
Yeah, good call. And this will benefit the MCP server once we expose it there.
There was a problem hiding this comment.
Now that it;s a public API, we should probably add JSDocs to the
ThemeProvidercomponentYeah, good call. And this will benefit the MCP server once we expose it there.
Added in b7cdc2f. Feedback welcome on wording. I originally started from how we describe it in README.md but refined it a bit. Maybe we want to revisit the README phrasing as well. "provide design tokens" isn't exactly accurate, because the provider doesn't generate a full set of tokens.
I also altered the ESLint configuration in 2158b04 to support this, and add a more complex explanation of why that exclusion applies.
I overlooked that and my instinct would be to remove it, as the intention here is stabilizing the API. What do you think? |
Given that we're potentially going to apply breaking changes to the DS tokens in the upcoming weeks, we could leave it around and remove it only when we stabilize the whole package (ie. version 1.0?), leaving this PR focused on making |
Yeah, that makes sense. It was also an oversight on my part that this doesn't do anything toward actually marking the package for 1.0 release, so we could make the promotion to 1.x and removal of the "experimental" banner a separate follow-on task closer to the 7.1 public release. Maybe tying it with a specific milestone like beta or RC ? |
b43c2be to
b7cdc2f
Compare
b7cdc2f to
f1c017b
Compare
|
@ciampo Do you think we're good to move forward with this, or were you considering this blocked by other theme stabilization work? |
|
#79254 should be the last PR introducing breaking changes in the tokens, should we wait for it to be merged first? |
No current external usage, ThemeProvider should generally be preferred.
7c79f96 to
9497e61
Compare
|
@aduth do you think we still need to
from the package? |
|
Also, it looks like |
Do you think we shouldn't? Personally I don't view that as related to the
Good catch, I'll update that 👍 |
I agree that they can provide value, but (especially given the nature of the package) I think we should only export what we know is needed as of now, and add more exports if necessary. The opposite won't be possible once the package is marked as stable |
We could still make backwards-incompatible changes to TypeScript typings, no? Since they're not part of the |
Not sure, we received complaints in the past about breaking type changes, too — especially form the point of view of third party maintainers that need to support multiple versions of WP and therefore woulnd't be able to have a single implementation that satisfies all versions at the same time (cc @mirka) |
|
That might be improved separately with #79455. In any case, let's take a look. I know we use a couple of the types ourselves at the moment, but most of them are likely unused. |
What?
Closes #76840
Closes #75319
Updates
@wordpress/themeto exportThemeProvideranduseThemeProviderStylesas part of its stable API.Why?
ThemeProviderstabilization is an expected deliverable of WordPress 7.1How?
lock-ing of private APIs from@wordpress/themeunlock-ing private APIs with direct import of the stable membersTesting Instructions
Verify no regressions in surfaces where ThemeProvider is used, for example the Fonts screen or Storybook Theme pages.
In Storybook:
npm run storybook:devIn Fonts screen:
npm run devUse of AI Tools
No AI was used.