-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Experiments: sharing private APIs with lock() and unlock() #46131
Conversation
Size Change: +6.28 kB (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
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.
Just noting that unless I'm missing something, this approach won't work for props, function arguments, keys in block.json, theme.json.
@youknowriad it will work for anything you can export – see my comments. I also added some examples to the contributing docs. As for the |
Also CCing @jsnajdr for comments |
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.
I like how this works for exported functions, components but I don't think it solves the problem for selectors, actions, props, function arguments, theme.json, block.json yet. (see my comments)
In other words, for now at least, I'm ok with this being the approach to take for exported functions and components but until we figure out the rest, we should be able to continue with our current approach. (would love if the documentation is updated as such accordingly)
@youknowriad I'll update the documentation and I agree about |
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.
I think this looking good, only thing for me is that I don't think we should be changing the policy for existing APIs and should remove this line. Thanks https://github.com/WordPress/gutenberg/pull/46131/files/c541a2c807a3a7e8af2f718e0ae99dabdb411829..05aac1d97a5ccce1cd5dac449db7593a68eee2d4#r1054431055
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.
No blockers from me any more. Thanks
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.
I'd love more opinions and thoughts about this PR as it's a departure from how we do things. So @WordPress/gutenberg-core should be aware of this.
Thanks for pushing through this.
This commit introduces a more convenient API for managing the private experiments. The idea is to "lock" private data inside public objects: ```js const { lock, unlock } = __dangerousOptInToUnstableAPIsOnlyForCoreModules( '<CONSENT STRING>', '@wordpress/blocks' ); export const publicObject = {}; lock( __experiments, "Shh, private data!" ); publicObject // {} unlock( publicObject ) // "Shh, private data!" ``` The lock()/unlock() API makes it easier to implement private functions, components, selectors, actions etc. Instead of using a separate import system, it's enough to opt-in to the experiments and then `unlock()` the publicly available artifact. For example: ```js import { store } from '@wordpress/blocks'; import { useSelect } from '@wordpress/data'; import { unlock } from './experiments'; function MyComponent() { const hasRole = useSelect( ( select ) => ( unlock( select( store ) ).__experimentalHasContentRoleAttribute() ) ); // ... } ``` This makes `unlock()` a drop-in replacement that does not require reworking the current code structure around the experimental APIs. What's also nice is that the above implementation of private selectors (coming in a follow-up PR) is short, sweet, and fairly easy to grasp. For comparison, an implementation based on the former register/unlock API required quite a bit of mental gymnastics: ```js // Package wordpress/block-editor: export const { register, unlock } = __dangerousOptInToUnstableAPIsOnlyForCoreModules( 'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.', '@wordpress/block-editor' ); const { __experimentalPrivateSelector } = unlock( dataExperiments ); import { __unstableSelectionHasUnmergeableBlock, } from './store/selectors'; export const __experimentalAccessKey = register( { __unstableSelectionHasUnmergeableBlock: __experimentalPrivateSelector( store, __unstableSelectionHasUnmergeableBlock ) } ); // Package wordpress/edit-page export const { register, unlock } = __dangerousOptInToUnstableAPIsOnlyForCoreModules( 'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.', '@wordpress/block-editor' ); import { __experimentalAccessKey as blockEditorExperiments } from '@wordpress/block-editor'; const { __unstableSelectionHasUnmergeableBlock } = unlock(__experimentalAccessKey); ``` It also had an undesirable side-effect of requiring a separate import and a different treatment of the `__unstableSelectionHasUnmergeableBlock()` selector – which is to say a migration would be rather difficult.
…on arguments, and react components
Co-authored-by: Robert Anderson <[email protected]>
…bleAPIsOnlyForCoreModules
… returned by unlock().
Nice work! |
Two notes so far from trying to use these APIs in the context of #47196:
|
@noisysocks Thank you for reporting! Both problems can be solved – let's discuss in #47229 and post the solution in this PR later on. Edit: Typehinting discussion |
To close the loop:
The TypeScript issue can be solved in a few different ways. Since folks are leaning towards locking all the experiments into a single exported // In the consumer package:
import { unlock } from './lock-unlock';
import { experiments as componentExperiments } from '@wordpress/components';
const { MyComponent } = unlock( componentExperiments );
// In the components package:
type Experiments = {
MyComponent: typeof ExperimentalMyComponent
};
export const experiments = {} as HasLockedType< Experiments >;
lock(experiments, {
MyComponent: ExperimentalMyComponent
});
// In the Experiments package:
type HasLockedType<Self, LockedType> = Self & { LockedType: LockedType };
type UnlockReturn< Arg > =
Arg extends {LockedType: infer T}
? T
: Arg;
function unlock<T>( arg: T ) : UnlockReturn<T> { /* ... */ }
@noisysocks's PR adds a new |
…wordpress/experiments (#47229) ## What? Part of #47196. Uses `@wordpress/experiments` (#46131) to make `__experimentalShowSelectedHint` in `CustomSelectControl` private. ## Why? We don't want to add any new experimental APIs to 6.2 as part of an effort to no longer expose experimental APIs in Core. ## How? https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/coding-guidelines.md#experimental-react-component-properties ## Testing Instructions 1. Use a block theme with more than 5 font sizes or manually edit `theme.json` to contain more than 5 font sizes in `settings.typography.fontSizes`. 2. Open the site editor. Appearance → Editor → Edit. 3. Go to Styles → Typography → Headings. 4. Select a heading level. 5. Toggle off the custom font size picker. 6. You should see a hint alongside the selected font size preset. Co-authored-by: Adam Zieliński <[email protected]>
…wordpress/experiments (#47229) ## What? Part of #47196. Uses `@wordpress/experiments` (#46131) to make `__experimentalShowSelectedHint` in `CustomSelectControl` private. ## Why? We don't want to add any new experimental APIs to 6.2 as part of an effort to no longer expose experimental APIs in Core. ## How? https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/coding-guidelines.md#experimental-react-component-properties ## Testing Instructions 1. Use a block theme with more than 5 font sizes or manually edit `theme.json` to contain more than 5 font sizes in `settings.typography.fontSizes`. 2. Open the site editor. Appearance → Editor → Edit. 3. Go to Styles → Typography → Headings. 4. Select a heading level. 5. Toggle off the custom font size picker. 6. You should see a hint alongside the selected font size preset. Co-authored-by: Adam Zieliński <[email protected]>
@adamziel, I just wanted to say that I quite like how this turned out, with the reduced surface and the proxy-powered unlocking. |
Gutenberg introduced a system of sharing private APIs in WordPress/gutenberg#46131. One of the safeguards is a check preventing the same module from opting-in twice so that contributors cannot easily gain access by pretending to be a core module. That safeguard is only meant for WordPress core and not for the released `@wordpress` packages. However, right now it is opt-out and must be explicitly disabled by developers wanting to install the `@wordpress` packages. Let's make it opt-out instead. This commit opts-out from that check in WordPress core by setting the ALLOW_EXPERIMENT_REREGISTRATION to false. Once it's merged, the Gutenberg plugin should be adjusted to use `true` as the default value.
…opt-out Gutenberg introduced a system of sharing private APIs in #46131. One of the safeguards is a check preventing the same module from opting-in twice so that contributors cannot easily gain access by pretending to be a core module. That safeguard is only meant for WordPress core and not for the released @WordPress packages. However, right now it is opt-out and must be explicitly disabled by developers wanting to install the @WordPress packages. Let's make it opt-out instead. This commit makes the check opt-in rather than opt-out. Its counterpart in the wordpress-develop repo makes WordPress explicitly set the ALLOW_EXPERIMENT_REREGISTRATION to false: WordPress/wordpress-develop#4121
…opt-out via IS_WORDPRESS_CORE (#48352) Gutenberg introduced a system of sharing private APIs in #46131. One of the safeguards is a check preventing the same module from opting-in twice so that contributors cannot easily gain access by pretending to be a core module. That safeguard is only meant for WordPress core and not for the released @WordPress packages. However, right now it is opt-out and must be explicitly disabled by developers wanting to install the @WordPress packages. Let's make it opt-out instead. In other words: * If we’re in WP core – prevent opting-in to private API with the same package name twice * If we’re not in WP core – don’t prevent it Or: * Before this commit, double opt-in safeguard is enabled by default * After this commit, double opt-in safeguard is disabled by default AND WordPress core [explicitly enables it](WordPress/wordpress-develop#4121) The corresponding PR in `wordpress-develop` repo makes WordPress explicitly set the `IS_WORDPRESS_CORE` to true: WordPress/wordpress-develop#4121
Description
This commit introduces a more convenient API for managing the private experiments as the former
register
-based API was quite cumbersome to use.The idea is to "lock" private data inside public objects:
This new
lock()
/unlock()
API enables private functions, classes, components, selectors, actions, arguments, and properties. Any package that opted-in to private APIs can callunlock()
on publicly available artifacts to retrieve the related private API.Kudos to @jsnajdr for identifying an opportunity to simplify the API!
Examples
Private selectors:
Private functions, classes, and variables
Private function arguments
To add an experimental argument to a stable function you'll need
to prepare a stable and an experimental version of that function.
Then, export the stable function and
lock()
the unstable functioninside it:
Private React Component properties
To add an experimental argument to a stable component you'll need
to prepare a stable and an experimental version of that component.
Then, export the stable function and
lock()
the unstable functioninside it:
Testing Instructions
Confirm the tests pass. Comment with your opinion on this API.
cc @dmsnell @gziolo @jsnajdr @jorgefilipecosta @priethor @georgeh @artemiomorales @tellthemachines @draganescu @peterwilsoncc