Repository navigation
Theme: Restore public ThemeProvider export #79620
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 14 commits
9fe7e18
9368815
3427a2d
a73e43e
c02f94b
9ee3bce
57715bd
ab8e694
5e1c566
4866d05
27733eb
a70f380
6d56aac
5d2a903
07a6f24
2c18621
86c2fc0
5e3a1ba
acc5316
4b2dcb5
9b0675e
cf72026
54eb383
1367e9d
2d11ce6
13cf1c4
0fbd30d
0ef91a0
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 |
|---|---|---|
| @@ -1,4 +1,11 @@ | ||
| import type { CornerRadiusPreset as InternalCornerRadiusPreset } from './types'; | ||
|
|
||
| export { privateApis } from './private-apis'; | ||
| export { ThemeProvider } from './theme-provider'; | ||
| export type * from './prebuilt/ts/token-types'; | ||
|
Contributor
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. Not sure whether we should keep these types around. On one hand, they are the TS reflection of the CSS tokens and they are consumed by In any case, changes such as adding a deprecation + lint errors can be handled in a separate PR.
Contributor
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 any case, given that they are consumed by
Member
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. I think we can/should keep these. It's different from component props, which can be extracted from the component exports themselves. |
||
| export { type ThemeProvider } from './theme-provider'; | ||
| export type { CornerRadiusPreset } from './types'; | ||
|
|
||
| /** | ||
| * @deprecated This type is not part of the intended public API and is scheduled | ||
| * for deletion as `@wordpress/theme` approaches stabilization. | ||
| */ | ||
| export type CornerRadiusPreset = InternalCornerRadiusPreset; | ||
|
Contributor
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. This is also a partially separate conversation to have. In short, we should not have a Given that this export was added recently (#78816) and didn't make it to a WP release yet, I wonder if we can be more aggressive and just remove it from the package directly, especially given that its only usages in the repo were "internal" ( cc @WordPress/gutenberg-components and @tyxla
Contributor
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. Update: I tentatively removed
Member
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. I'm inclined to remove it too. Particularly since the rationale you suggested in #78958 (comment) for why TypeScript types would need to be sensitive to backwards-compatibility has to do with plugins supporting multiple versions of WordPress. If this has never landed in a WordPress release, combined with the fact that TypeScript types don't ever get exposed on
Member
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. Agreed 👍 |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,27 @@ | ||
| import warning from '@wordpress/warning'; | ||
| import { lock } from './lock-unlock'; | ||
| import { ThemeProvider } from './theme-provider'; | ||
| import { useThemeProviderStyles } from './use-theme-provider-styles'; | ||
|
|
||
| function warnPrivateApi( apiName: string ) { | ||
| warning( | ||
|
ciampo marked this conversation as resolved.
Resolved
|
||
| `\`@wordpress/theme\`: Accessing \`${ apiName }\` through private APIs is deprecated. Import \`ThemeProvider\` from \`@wordpress/theme\` instead; this private export is scheduled for deletion as \`@wordpress/theme\` approaches stabilization.` | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @deprecated Private `@wordpress/theme` APIs are scheduled for deletion as the | ||
| * package approaches stabilization. Import public APIs from `@wordpress/theme` | ||
| * instead. | ||
| */ | ||
| export const privateApis = {}; | ||
| lock( privateApis, { | ||
| ThemeProvider, | ||
| useThemeProviderStyles, | ||
| get ThemeProvider() { | ||
| warnPrivateApi( 'ThemeProvider' ); | ||
| return ThemeProvider; | ||
| }, | ||
| get useThemeProviderStyles() { | ||
| warnPrivateApi( 'useThemeProviderStyles' ); | ||
| return useThemeProviderStyles; | ||
| }, | ||
| } ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import { unlock } from '../lock-unlock'; | ||
| import { privateApis } from '../private-apis'; | ||
| import { ThemeProvider } from '../theme-provider'; | ||
| import { useThemeProviderStyles } from '../use-theme-provider-styles'; | ||
|
|
||
| describe( 'privateApis', () => { | ||
| it( 'warns when accessing useThemeProviderStyles through private APIs', () => { | ||
| const warn = jest | ||
| .spyOn( console, 'warn' ) | ||
| .mockImplementation( () => {} ); | ||
|
|
||
| const unlockedPrivateApis = unlock< { | ||
| useThemeProviderStyles: typeof useThemeProviderStyles; | ||
| } >( privateApis ); | ||
|
|
||
| expect( unlockedPrivateApis.useThemeProviderStyles ).toBe( | ||
| useThemeProviderStyles | ||
| ); | ||
|
|
||
| expect( warn ).toHaveBeenCalledWith( | ||
| '`@wordpress/theme`: Accessing `useThemeProviderStyles` through private APIs is deprecated. Import `ThemeProvider` from `@wordpress/theme` instead; this private export is scheduled for deletion as `@wordpress/theme` approaches stabilization.' | ||
|
ciampo marked this conversation as resolved.
Resolved
|
||
| ); | ||
|
|
||
| warn.mockRestore(); | ||
| } ); | ||
|
|
||
| it( 'warns when accessing ThemeProvider through private APIs', () => { | ||
| const warn = jest | ||
| .spyOn( console, 'warn' ) | ||
| .mockImplementation( () => {} ); | ||
|
|
||
| const unlockedPrivateApis = unlock< { | ||
| ThemeProvider: typeof ThemeProvider; | ||
| } >( privateApis ); | ||
|
|
||
| expect( unlockedPrivateApis.ThemeProvider ).toBe( ThemeProvider ); | ||
|
|
||
| expect( warn ).toHaveBeenCalledWith( | ||
| '`@wordpress/theme`: Accessing `ThemeProvider` through private APIs is deprecated. Import `ThemeProvider` from `@wordpress/theme` instead; this private export is scheduled for deletion as `@wordpress/theme` approaches stabilization.' | ||
| ); | ||
|
|
||
| warn.mockRestore(); | ||
| } ); | ||
| } ); | ||
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.