Skip to content
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

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

Merged
merged 4 commits into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1607,6 +1607,12 @@
"markdown_source": "../packages/eslint-plugin/README.md",
"parent": "packages"
},
{
"title": "@wordpress/experiments",
"slug": "packages-experiments",
"markdown_source": "../packages/experiments/README.md",
"parent": "packages"
},
{
"title": "@wordpress/format-library",
"slug": "packages-format-library",
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"@wordpress/editor": "file:packages/editor",
"@wordpress/element": "file:packages/element",
"@wordpress/escape-html": "file:packages/escape-html",
"@wordpress/experiments": "file:packages/experiments",
"@wordpress/format-library": "file:packages/format-library",
"@wordpress/hooks": "file:packages/hooks",
"@wordpress/html-entities": "file:packages/html-entities",
Expand Down
35 changes: 35 additions & 0 deletions packages/experiments/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "@wordpress/dependency-injection",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"version": "0.0.1",
"description": "Dependency Injection container for WordPress.",
"author": "The WordPress Contributors",
"license": "GPL-2.0-or-later",
"keywords": [
"wordpress",
"gutenberg",
"dom",
"utils"
],
"homepage": "https://github.com/WordPress/gutenberg/tree/HEAD/packages/dependency-injection/README.md",
"repository": {
"type": "git",
"url": "https://github.com/WordPress/gutenberg.git",
"directory": "packages/injection"
},
"bugs": {
"url": "https://github.com/WordPress/gutenberg/issues"
},
"engines": {
"node": ">=12"
},
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": false,
"dependencies": {
"@babel/runtime": "^7.16.0"
},
"publishConfig": {
"access": "public"
}
}
85 changes: 85 additions & 0 deletions packages/experiments/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
const CORE_MODULES_USING_EXPERIMENTS = [
'@wordpress/data',
'@wordpress/block-editor',
'@wordpress/block-library',
'@wordpress/blocks',
'@wordpress/core-data',
'@wordpress/date',
'@wordpress/edit-site',
'@wordpress/edit-widgets',
];

const registeredExperiments = {};
/*
* Warning for theme and plugin developers.
*
* The use of experimental developer APIs is intended for use by WordPress Core
* and the Gutenberg plugin exclusively.
*
* Dangerously opting in to using these APIs is NOT RECOMMENDED. Furthermore,
* the WordPress Core philosophy to strive to maintain backward compatibility
* for third-party developers DOES NOT APPLY to experimental APIs.
*
* THE CONSENT STRING FOR OPTING IN TO THESE APIS MAY CHANGE AT ANY TIME AND
* WITHOUT NOTICE. THIS CHANGE WILL BREAK EXISTING THIRD-PARTY CODE. SUCH A
* CHANGE MAY OCCUR IN EITHER A MAJOR OR MINOR RELEASE.
*/
const requiredConsent =
'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.';
adamziel marked this conversation as resolved.
Show resolved Hide resolved

export const __dangerousOptInToUnstableAPIsOnlyForCoreModules = (
consent,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately no :-(

Copy link
Contributor Author

@adamziel adamziel Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
   ...
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

moduleName
) => {
if ( ! CORE_MODULES_USING_EXPERIMENTS.includes( moduleName ) ) {
throw new Error(
adamziel marked this conversation as resolved.
Show resolved Hide resolved
`You tried to opt-in to unstable APIs as a module "${ moduleName }". ` +
'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 be removed ' +
'without a warning. If you ignore this error and depend on unstable features, ' +
'your product will inevitably break on one of the next WordPress releases.'
);
}
if ( moduleName in registeredExperiments ) {
throw new Error(
`You tried to opt-in to unstable APIs as a module "${ moduleName }" which is already registered. ` +
'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 be removed ' +
'without a warning. If you ignore this error and depend on unstable features, ' +
'your product will inevitably break on one of the next WordPress releases.'
);
}
if ( consent !== requiredConsent ) {
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 ' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this message a string and reuse it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes :D

'without a warning. If you ignore this error and depend on unstable features, ' +
'your product will inevitably break on the next WordPress release.'
);
}
registeredExperiments[ moduleName ] = {
accessKey: {},
apis: {},
};
return {
register: ( experiments ) => {
for ( const key in experiments ) {
registeredExperiments[ moduleName ].apis[ key ] =
experiments[ key ];
}
return registeredExperiments[ moduleName ].accessKey;
},
unlock: ( accessKey ) => {
for ( const experiment of Object.values( registeredExperiments ) ) {
if ( experiment.accessKey === accessKey ) {
return experiment.apis;
}
}

throw new Error(
'There is no registered module matching the specified access key'
);
},
};
};
84 changes: 84 additions & 0 deletions packages/experiments/src/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* Internal dependencies
*/
import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '../';

const requiredConsent =
'I know using unstable features means my plugin or theme will inevitably break on the next WordPress release.';

describe( '__dangerousOptInToUnstableAPIsOnlyForCoreModules', () => {
it( 'Should require a consent string', () => {
expect( () => {
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
'',
'@wordpress/data'
);
} ).toThrow( /without confirming you know the consequences/ );
} );
it( 'Should require a valid @wordpress package name', () => {
expect( () => {
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'custom_package'
);
} ).toThrow(
/This feature is only for JavaScript modules shipped with WordPress core/
);
} );
it( 'Should not register the same module twice', () => {
expect( () => {
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'@wordpress/edit-widgets'
);
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'@wordpress/edit-widgets'
);
} ).toThrow( /is already registered/ );
} );
it( 'Should grant access to unstable APIs when passed both a consent string and a previously unregistered package name', () => {
const unstableAPIs = __dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'@wordpress/edit-site'
);
expect( unstableAPIs.unlock ).toEqual( expect.any( Function ) );
expect( unstableAPIs.register ).toEqual( expect.any( Function ) );
} );
it( 'Should register and unlock experimental APIs', () => {
// This would live in @wordpress/data:
// Opt-in to experimental APIs
const dataExperiments =
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'@wordpress/data'
);

// Register the experimental APIs
const dataExperimentalFunctions = {
__experimentalFunction: jest.fn(),
};
const dataAccessKey = dataExperiments.register(
dataExperimentalFunctions
);

// This would live in @wordpress/core-data:
// Register the experimental APIs
const coreDataExperiments =
__dangerousOptInToUnstableAPIsOnlyForCoreModules(
requiredConsent,
'@wordpress/core-data'
);

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

// Call one!
__experimentalFunction();

expect(
dataExperimentalFunctions.__experimentalFunction
).toHaveBeenCalled();
} );
} );