Limit access to experimental APIs to WordPress codebase with a new experiments package #43386

adamziel · 2022-08-18T13:43:46Z

What

This PR proposes a way of making the __experimental APIs private as exposing them to publicly plugin authors is quite controversial..

Tl;dr, the idea is to have a "dealer" that only deals the experimental APIs to WordPress packages.

Each package would start by registering itself:

const { register } = 
	__dangerousOptInToUnstableAPIsOnlyForCoreModules(
		'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
		'@wordpress/blocks'
	);

The function name communicates that plugins are not supposed to use it. To make double and triple sure, the first argument must be that exact consent string, and the second argument must be a known @wordpress package that hasn't opted in yet – otherwise an error is thrown.

Exposing a new __experimental API looks like this:

export __experiments = register( { __unstableGetInnerBlocksProps } )

Now here's the interesting part – consuming a registered __experimental API requires opting-in first:

// In the @wordpress/block-editor package:
import { __experiments as blocksExperiments } from '@wordpress/blocks';
const { unlock } = 
	__dangerousOptInToUnstableAPIsOnlyForCoreModules(
		'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
		'@wordpress/block-editor'
	);

const { __unstableGetInnerBlocksProps } = unlock( blocksExperiments );

window.wp is still the recommended way of using the public APIs. A determined developer who would want to use the experimental APIs at all costs would have to:

Realize a private importing system exists
Read the code where the risks would be spelled out in capital letters
Explicitly type out he or she is aware of the consequences
Pretend to register a @wordpress package (and trigger an error as soon as the real package is loaded)

This PR touches 21 files but the important part is the new @wordpress/experiments module. Everything else is just a demo that I want to remove before merging this PR. There is no need to migrate the existing __experimental APIs to this new system as they are already being publicly exported. Only the future __experimental APIs would use this system.

Notable usage examples

The usage examples previously shipped with this PR include:

@wordpress/data offers experimental utils for private actions and selectors
__experimentalReapplyBlockTypeFilters() is one such private action – and here's where it's used.
@wordpress/edit-widgets uses experiments registered by three different packages
@wordpress/data only registers experimental APIs without requesting any

See the entire (now removed) commit tree.

This PR is an alternative to #41278

Testing Instructions

Ignore the CI. Let's focus on the discussion – what do you like and dislike about this idea?

CC @gziolo @peterwilsoncc @mcsf @talldan @noisysocks @azaozz @jorgefilipecosta @mtias

packages/block-editor/src/experiments.js

packages/experiments/src/index.js

peterwilsoncc · 2022-08-23T03:09:27Z

packages/experiments/src/index.js

+	'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.';
+
+export const __dangerousOptInToUnstableAPIsOnlyForCoreModules = (
+	consent,


Is it possible to ensure calls to this function use a string (as opposed to a variable set to a string).

// Allowed __dangerousOptInToUnstableAPIsOnlyForCoreModules( 'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.', ... ); // Not allowed const yeahWhatever = 'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.'; __dangerousOptInToUnstableAPIsOnlyForCoreModules( yeahWhatever, ... );

I suspect the answer is no but would rather check :)

Unfortunately no :-(

Although this is be possible:

import { __dangerousOptInToUnstableAPIsOnlyForCoreModules, _i_know_using_unstable_features_means_my_plugin_or_theme_will_inevitably_break_on_the_next_WordPress_release } from '@wordpress/experiments'; __dangerousOptInToUnstableAPIsOnlyForCoreModules( _i_know_using_unstable_features_means_my_plugin_or_theme_will_inevitably_break_on_the_next_WordPress_release, ... )

Loving the huge, awkward consent message 😂

Going the importable variable route would definitely make things easier. I don't know if we want to make things easier? But either way, we'll end up copying and pasting the message in, so the added difficulty is only in locating a place to copy from.

packages/experiments/src/index.js

draganescu · 2022-08-23T12:10:05Z

packages/experiments/src/index.js

+		throw new Error(
+			`You tried to opt-in to unstable APIs without confirming you know the consequences. ` +
+				'This feature is only for JavaScript modules shipped with WordPress core. ' +
+				'Please do not use it in plugins and themes as the unstable APIs will removed ' +


Can you make this message a string and reuse it?

draganescu

I find this solution to be pretty good, a wrapper package acting as a cloak around experiments and practically hiding them from a generic global access point.

I think the opt in can be less verbose - we're a bit over doing it, but the technical approach looks good enough to me. TBH this is more than enough for what is a best practice anyway.

gziolo · 2022-08-23T14:49:08Z

I think the opt in can be less verbose - we're a bit over doing it, but the technical approach looks good enough to me. TBH this is more than enough for what is a best practice anyway.

Agreed, however it is perfect to show how far we can go to highlight that experiments are meant to be for internal usage only.

jorgefilipecosta

This seems like a solution that fits our needs, thank you for working on it @adamziel!

There are some cases where using this solution is not so direct:

Experimental properties in a component.
Experimental keys in a "settings" object.
Experimental parameters in a function.
Experimental block registration settings in block.json (e.g:__experimentalSelector, __experimentalRole)

I guess for some cases like the experimental properties in a component, we can expose two components, a stable and an Experimental one, where Experimental properties are only supported on the Experimental component. The same for experimental parameters in a function. It may be tricky because sometimes these experimental properties are passed down multiple levels, so we need to create many experimental components/functions just because it is passing down a property, but I think it is acceptable.
For things like experimental block.json properties, the scenario is more complex. Maybe we can have an experimental function that needs to be called when using experimental block.json properties or something similar.

adamziel · 2022-08-23T15:51:33Z

Thank you for reviewing @jorgefilipecosta! As you say, there are some other tricky cases to handle, but this at least gets us moving.

I guess for some cases like the experimental properties in a component, we can expose two components, a stable and an Experimental one, where Experimental properties are only supported on the Experimental component.

I think you're on to something here!

Experimental parameters in a function.

The publicly exported function would have no __experimental parameters in the signature, and the private one would have them. For example:

const __experimentalDoGreatThings = ( id, __experimentalArgument ) => {
    if( __experimentalArgument ) {
        // ... do something ...
    }
}

export const doGreatThings = ( id ) => __experimentalDoGreatThings( id, false );
registerExperimentalAPI({
    __experimentalDoGreatThings
});

Experimental properties in a component.

Most components are just functions so your idea still applies!

Sometimes these experimental properties are passed down multiple levels

Core components can render the experimental versions by default without adding too much complexity:

const __experimentalRoot = ({ id, __experimentalArgument }) => {
    return <__experimentalBranch id={id} __experimentalArgument={__experimentalArgument} />
}
const __experimentalBranch = ({ id, __experimentalArgument }) => {
    return <__experimentalLeaf id={id} __experimentalArgument={__experimentalArgument} />
}
const __experimentalLeaf = ({ id, __experimentalArgument }) => {
    // ...
}

export const Root = ({ id }) => __experimentalRoot({ id });
export const Branch = ({ id }) => __experimentalBranch({ id });
export const Leaf = ({ id }) => __experimentalLeaf({ id });

registerExperimentalAPI({
    __experimentalRoot,
    __experimentalBranch,
    __experimentalLeaf
});

Experimental keys in a "settings" object.

I can't think of any way around that, although we could make it harder and don't accept these in PHP setup functions.

For things like experimental block.json properties, the scenario is more complex. Maybe we can have an experimental function that needs to be called when using experimental block.json properties or something similar.

Since WordPress has a baked-in list of core blocks, perhaps the idea below would work?

function parseBlockJson( $string ) {
    $data = json_decode( $string );
    if( ! is_core_block( $data['name'] ) ) {
        $data = remove_experimental_properties( $data );
    }
    // ...
}

tellthemachines

Thanks for looking into this, looks like a solid start to addressing the issue!

I guess for some cases like the experimental properties in a component, we can expose two components, a stable and an Experimental one, where Experimental properties are only supported on the Experimental component. The same for experimental parameters in a function. It may be tricky because sometimes these experimental properties are passed down multiple levels, so we need to create many experimental components/functions just because it is passing down a property, but I think it is acceptable.

Yeah, this is an interesting problem. I wonder if the added complexity to creating experimental parameters might result in us doing so less often. There's no rule about changes of that sort having to be marked experimental. Or should we try to enforce anything along those lines too?

packages/block-editor/src/components/inner-blocks/index.js

tellthemachines · 2022-08-24T01:35:04Z

packages/experiments/src/index.js

+	'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.';
+
+export const __dangerousOptInToUnstableAPIsOnlyForCoreModules = (
+	consent,


Loving the huge, awkward consent message 😂

Going the importable variable route would definitely make things easier. I don't know if we want to make things easier? But either way, we'll end up copying and pasting the message in, so the added difficulty is only in locating a place to copy from.

adamziel · 2022-08-30T14:29:22Z

Going the importable variable route would definitely make things easier. I don't know if we want to make things easier? But either way, we'll end up copying and pasting the message in, so the added difficulty is only in locating a place to copy from.

@tellthemachines that's good IMHO, you can't use this without inspecting the code and realizing why you are not supposed to do that.

adamziel · 2022-08-30T14:31:03Z

@peterwilsoncc @azaozz @Clorith @tradesouthwest can you spot any problems with the approach proposed here? If not, let's move forward with this.

gziolo · 2022-08-30T14:38:17Z

My understanding is that this proposal addresses all concerns discussed in the earlier proposal #41278. It allows to expose code between scripts without exposing it in the public API. The added benefit is that we will have a single place to update if we want to collect details on the number and longevity of the experiments run. To the last point, I would be happy to see the required Gutenberg and/or version number when the API was introduced.

peterwilsoncc

I am so up for this. I've added a suggestion for a fairly unambiguous comment. I'm not sure if it's best as a comment or as a part of the docblock for the dangerous export function.

It's not very subtle :)

packages/experiments/src/index.js

gziolo · 2022-09-08T12:16:21Z

One thing I don't like about this proposal is how it works with the module system. To use experiments from @wordpress/data for example, nobody is really forcing me to import @wordpress/data. I just rely on someone doing the (side-effectful) import '@wordpress/data' for me and triggering registration with that.

Great feedback! We definitely need to take that into account so we can enforce that the API is already registered 👍🏻

Would it work with a simpler API like this?

import { __experimentalUnlock } from '@wordpress/data';

const { doExperimentalDataStuff } = __experimentalUnlock( secretTokenOrSomethingElseFromTheCurrentPackage );

jsnajdr · 2022-09-08T12:37:55Z

Would it work with a simpler API like this?

I don't understand this example. Should it look more like this?

import { __experimentalUnlock } from '@wordpress/experiments';
import { secretTokenOrSomethingElseFromTheCurrentPackage } from '@wordpress/data';
const { doExperimentalDataStuff } = __experimentalUnlock( secretTokenOrSomethingElseFromTheCurrentPackage );

…rimental APIs as dependencies

adamziel · 2022-09-26T02:52:04Z

@jsnajdr I went for the following API – what do you think?

// in @wordpress/data
import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/experiments';
const experiments = __dangerousOptInToUnstableAPIsOnlyForCoreModules(
	'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
	'@wordpress/data'
);

export const __experiments = experiments.register({
	__experimentalFunction: () => { /* ... */ },
});

// In @wordpress/core-data:
import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/experiments';
import { __experiments as __dataExperiments } from '@wordpress/data';

const experiments = __dangerousOptInToUnstableAPIsOnlyForCoreModules(
	'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.',
	'@wordpress/core-data'
);

// Get the experimental APIs registered by @wordpress/data
const { __experimentalFunction } = experiments.unlock( __dataExperiments );

__experimentalFunction();

Here the unlock helper has a potential to be used universally to access experimental exports of a module, to convert a function without experimental params to function with experimental params, and generally to link together two versions of a thing: one without experimental stuff, and the other with it, accessed with unlock( ... ).

I like it!

gziolo · 2022-09-26T05:11:31Z

@adamziel, so you always return only access keys (unlock needs to remain private) from WordPress packages that expose experimental APIs. It means that only packages that opt-in for experimental APIs can use those access keys. We will maintain the list of those packages that can register and/or access experiments to lock it down to WordPress core. If I understood the code correctly then this is good to go 💯

adamziel · 2022-09-26T06:37:37Z

Thanks @gziolo! I went ahead and merged. Since this code is internal by definition, we can always adjust it in a follow-up PR.

gziolo · 2022-09-26T07:04:59Z

packages/experiments/package.json

@@ -0,0 +1,35 @@
+{
+	"name": "@wordpress/dependency-injection",


I have just noticed that it should be @wordpress/experiments before we start using it.

gziolo · 2022-09-26T07:07:28Z

As a follow-up, we need to add README.md and CHANGELOG.md files. We also should either rename the folder with the package to dependency-injection or update this name with experiments in package.json file.

talldan · 2022-09-28T04:40:37Z

Thanks for seeing this through Adam. I haven't had much chance to offer feedback, but it's encouraging to see some solutions coming together.

It'd be great to also add or update contributor docs in addition to the readme:
https://github.com/WordPress/gutenberg/tree/trunk/docs/contributors/code

Even if it's just a brief summary with a link to the package's readme.

adamziel · 2022-09-28T11:08:09Z

Great idea @talldan 💯 , I'll look into that soon. For now I started exploring the integration in #44521.

jsnajdr · 2022-09-29T09:09:47Z

I went for the following API – what do you think?

I only got to this now, let me share some observations:

The unlock function -- is it supposed to open any module? Because in the current incarnation, I'm getting a reference to the function by opting in to one particular module:

const { unlock } = __dangerouslyOptIn( 'I know', '@wordpress/data' );

but then can use it to unlock any other module, too:

import { __blockEditorExperiments } from '@wordpress/block-editor';
const { doBlockEditorStuff } = unlock( __blockEditorExperiments );

Is that desired? I would say it isn't, because I dangerously opted in and consented only to the @wordpress/data module, not any other one.

If omniponent unlock is really what we want, then we don't need to create an unlock function for each module, like:

return {
  register: ( experiments ) => { ... },
  unlock: ( key ) => { ... },
}

but can have one global shared one

function unlock( key ) {
  ...
}
return {
  register: ( experiments ) => { ... },
  unlock,
}

That's just a little clarification/optimization, nothing else.

In #44521 I also see a little confusion about naming the "access key". Sometimes it's like dataExperiments, sometimes it's like experimentsAccessKey. In the mental model of the experiments system, is it a "key", like in a key-value map, or is it already the experiments object, but just in an opaque form that needs to be unlocked?

I like the latter, when we treat it as the object itself, and are just unlocking it.

Introduces a private selectors APIs in `@wordpress/data` via [the new `@wordpress/experimental`](#43386 (comment)) package: ```js // Package wordpress/block-data: import { unlock } from '../experiments'; import { experiments as dataExperiments } from '@wordpress/data'; const { registerPrivateActionsAndSelectors } = unlock( dataExperiments ); import { store as blockEditorStore } from './store'; import { __unstableSelectionHasUnmergeableBlock } from './store/selectors'; registerPrivateActionsAndSelectors( store, {}, { __experimentalHasContentRoleAttribute } ); // plain usage unlock( registry.select( blockEditorStore ) ).getContentLockingParent(); // usage in React useSelect( ( select ) => ( { parent: privateOf( select( blockEditorStore ) ).__unstableSelectionHasUnmergeableBlock(); } ) ); ```

bph · 2023-02-05T16:00:29Z

Added Needs Dev Note to get it into the Fieldguide for 6.2

gziolo · 2023-02-06T08:00:08Z

It's fully documented in this place:

https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/coding-guidelines.md#experimental-and-unstable-apis

It doesn't need to be necessarily treated as a dev note, but it surely deserves an update on the Make Core blog.

It might also be a good idea to consider changing the sentiment for legacy experimental and unstable APIs, as I don't think we want them to use them anymore now that we have a proper API to achieve the same goal.

adamziel self-assigned this Aug 18, 2022

adamziel added [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible Developer Experience Ideas about improving block and theme developer experience labels Aug 18, 2022

adamziel mentioned this pull request Aug 18, 2022

Plugin: Try API for making experiments limited to WordPress codebase #41278

Closed

adamziel marked this pull request as ready for review August 19, 2022 09:23

adamziel requested review from draganescu, talldan, noisysocks, tellthemachines, kevin940726, ellatrix, ajitbohra and nerrad as code owners August 19, 2022 09:23

gziolo reviewed Aug 19, 2022

View reviewed changes

packages/block-editor/src/experiments.js Outdated Show resolved Hide resolved

gziolo reviewed Aug 19, 2022

View reviewed changes

packages/experiments/src/index.js Outdated Show resolved Hide resolved

peterwilsoncc reviewed Aug 23, 2022

View reviewed changes

draganescu reviewed Aug 23, 2022

View reviewed changes

packages/experiments/src/index.js Outdated Show resolved Hide resolved

draganescu reviewed Aug 23, 2022

View reviewed changes

packages/experiments/src/index.js Outdated Show resolved Hide resolved

draganescu reviewed Aug 23, 2022

View reviewed changes

packages/experiments/src/index.js Outdated Show resolved Hide resolved

draganescu reviewed Aug 23, 2022

View reviewed changes

packages/experiments/src/index.js Outdated Show resolved Hide resolved

draganescu reviewed Aug 23, 2022

View reviewed changes

jorgefilipecosta reviewed Aug 23, 2022

View reviewed changes

tellthemachines reviewed Aug 24, 2022

View reviewed changes

peterwilsoncc reviewed Aug 30, 2022

View reviewed changes

packages/experiments/src/index.js Show resolved Hide resolved

adamziel added 3 commits September 26, 2022 12:42

Revert all demo changes and leave just the experiments package

0157c34

Update package-lock.json and docs/manifest.json

c0688d8

Redesign the API to require listing the packages registering the expe…

3cc920f

…rimental APIs as dependencies

adamziel force-pushed the explore/experimental branch from 056742a to 3cc920f Compare September 26, 2022 02:42

Shorten the methods names

bf47c9b

adamziel merged commit ace58e4 into trunk Sep 26, 2022

adamziel deleted the explore/experimental branch September 26, 2022 06:36

github-actions bot added this to the Gutenberg 14.3 milestone Sep 26, 2022

gziolo reviewed Sep 26, 2022

View reviewed changes

adamziel mentioned this pull request Sep 26, 2022

[experiments] Add README.md and CHANGELOG.md #44457

Merged

adamziel mentioned this pull request Sep 28, 2022

Private experimental cross-module selectors and actions #44521

Merged

adamziel changed the title ~~Limit access to experimental APIs to WordPress codebase~~ Limit access to experimental APIs to WordPress codebase with a new experiments package Oct 28, 2022

bph added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Feb 5, 2023

bph mentioned this pull request Feb 6, 2023

6.2 Dev Note Tracking Issue #47771

Closed

47 tasks

adamziel added has dev note when dev note is done (for upcoming WordPress release) and removed Needs Dev Note Requires a developer note for a major WordPress release cycle has dev note when dev note is done (for upcoming WordPress release) labels Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit access to experimental APIs to WordPress codebase with a new experiments package #43386

Limit access to experimental APIs to WordPress codebase with a new experiments package #43386

adamziel commented Aug 18, 2022 •

edited

Loading

peterwilsoncc Aug 23, 2022

adamziel Aug 23, 2022

adamziel Aug 23, 2022 •

edited

Loading

tellthemachines Aug 24, 2022

draganescu Aug 23, 2022

adamziel Aug 23, 2022

draganescu left a comment

gziolo commented Aug 23, 2022

jorgefilipecosta left a comment

adamziel commented Aug 23, 2022 •

edited

Loading

tellthemachines left a comment

tellthemachines Aug 24, 2022

adamziel commented Aug 30, 2022

adamziel commented Aug 30, 2022

gziolo commented Aug 30, 2022

peterwilsoncc left a comment

gziolo commented Sep 8, 2022 •

edited

Loading

jsnajdr commented Sep 8, 2022

adamziel commented Sep 26, 2022 •

edited

Loading

gziolo commented Sep 26, 2022

adamziel commented Sep 26, 2022 •

edited

Loading

gziolo Sep 26, 2022

gziolo commented Sep 26, 2022

talldan commented Sep 28, 2022

adamziel commented Sep 28, 2022 •

edited

Loading

jsnajdr commented Sep 29, 2022

bph commented Feb 5, 2023

gziolo commented Feb 6, 2023

Limit access to experimental APIs to WordPress codebase with a new experiments package #43386

Limit access to experimental APIs to WordPress codebase with a new experiments package #43386

Conversation

adamziel commented Aug 18, 2022 • edited Loading

What

Notable usage examples

Testing Instructions

peterwilsoncc Aug 23, 2022

Choose a reason for hiding this comment

adamziel Aug 23, 2022

Choose a reason for hiding this comment

adamziel Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

tellthemachines Aug 24, 2022

Choose a reason for hiding this comment

draganescu Aug 23, 2022

Choose a reason for hiding this comment

adamziel Aug 23, 2022

Choose a reason for hiding this comment

draganescu left a comment

Choose a reason for hiding this comment

gziolo commented Aug 23, 2022

jorgefilipecosta left a comment

Choose a reason for hiding this comment

adamziel commented Aug 23, 2022 • edited Loading

tellthemachines left a comment

Choose a reason for hiding this comment

tellthemachines Aug 24, 2022

Choose a reason for hiding this comment

adamziel commented Aug 30, 2022

adamziel commented Aug 30, 2022

gziolo commented Aug 30, 2022

peterwilsoncc left a comment

Choose a reason for hiding this comment

gziolo commented Sep 8, 2022 • edited Loading

jsnajdr commented Sep 8, 2022

adamziel commented Sep 26, 2022 • edited Loading

gziolo commented Sep 26, 2022

adamziel commented Sep 26, 2022 • edited Loading

gziolo Sep 26, 2022

Choose a reason for hiding this comment

gziolo commented Sep 26, 2022

talldan commented Sep 28, 2022

adamziel commented Sep 28, 2022 • edited Loading

jsnajdr commented Sep 29, 2022

bph commented Feb 5, 2023

gziolo commented Feb 6, 2023

adamziel commented Aug 18, 2022 •

edited

Loading

adamziel Aug 23, 2022 •

edited

Loading

adamziel commented Aug 23, 2022 •

edited

Loading

gziolo commented Sep 8, 2022 •

edited

Loading

adamziel commented Sep 26, 2022 •

edited

Loading

adamziel commented Sep 26, 2022 •

edited

Loading

adamziel commented Sep 28, 2022 •

edited

Loading