Skip to content

Commit

Permalink
[Private APIs] Make the re-registration safeguard opt-in rather than …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
adamziel authored Feb 28, 2023
1 parent 2a52f30 commit 09eeb8c
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 29 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
"npm": ">=6.9.0 <7"
},
"config": {
"IS_GUTENBERG_PLUGIN": true,
"ALLOW_EXPERIMENT_REREGISTRATION": true
"IS_GUTENBERG_PLUGIN": true
},
"dependencies": {
"@wordpress/a11y": "file:packages/a11y",
Expand Down
11 changes: 7 additions & 4 deletions packages/private-apis/src/implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ const requiredConsent =

/** @type {boolean} */
let allowReRegistration;
// Use try/catch to force "false" if the environment variable is not explicitly
// set to true (e.g. when building WordPress core).
// The safety measure is meant for WordPress core where IS_WORDPRESS_CORE
// is set to true.
// For the general use-case, the re-registration should be allowed by default
// Let's default to true, then. Try/catch will fall back to "true" even if the
// environment variable is not explicitly defined.
try {
allowReRegistration = process.env.ALLOW_EXPERIMENT_REREGISTRATION ?? false;
allowReRegistration = process.env.IS_WORDPRESS_CORE ? false : true;
} catch ( error ) {
allowReRegistration = false;
allowReRegistration = true;
}

/**
Expand Down
6 changes: 0 additions & 6 deletions storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,4 @@ module.exports = {
emotionAlias: false,
storyStoreV7: true,
},
env: ( config ) => ( {
...config,
// Inject the `ALLOW_EXPERIMENT_REREGISTRATION` global, used by
// @wordpress/private-apis.
ALLOW_EXPERIMENT_REREGISTRATION: true,
} ),
};
8 changes: 0 additions & 8 deletions test/native/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ import { Image, NativeModules as RNNativeModules } from 'react-native';
// testing environment: https://github.com/facebook/react-native/blob/6c19dc3266b84f47a076b647a1c93b3c3b69d2c5/Libraries/Core/setUpNavigator.js#L17
global.navigator = global.navigator ?? {};

/**
* Whether to allow the same experiment to be registered multiple times.
* This is useful for development purposes, but should be set to false
* during the unit tests to ensure the Gutenberg plugin can be cleanly
* merged into WordPress core where this is false.
*/
global.process.env.ALLOW_EXPERIMENT_REREGISTRATION = true;

// Set up the app runtime globals for the test environment, which includes
// modifying the above `global.navigator`
require( '../../packages/react-native-editor/src/globals' );
Expand Down
10 changes: 5 additions & 5 deletions test/unit/config/gutenberg-env.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ global.process.env = {
IS_GUTENBERG_PLUGIN:
String( process.env.npm_package_config_IS_GUTENBERG_PLUGIN ) === 'true',
/**
* Whether to allow the same experiment to be registered multiple times.
* This is useful for development purposes, but should be set to false
* during the unit tests to ensure the Gutenberg plugin can be cleanly
* merged into WordPress core where this is false.
* Feature flag guarding features specific to WordPress core.
* It's important to set it to "true" in the test environment
* to ensure the Gutenberg plugin can be cleanly merged into
* WordPress core.
*/
ALLOW_EXPERIMENT_REREGISTRATION: false,
IS_WORDPRESS_CORE: true,
};
6 changes: 3 additions & 3 deletions tools/webpack/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ const plugins = [
// Inject the `IS_GUTENBERG_PLUGIN` global, used for feature flagging.
'process.env.IS_GUTENBERG_PLUGIN':
process.env.npm_package_config_IS_GUTENBERG_PLUGIN,
// Inject the `ALLOW_EXPERIMENT_REREGISTRATION` global, used by @wordpress/private-apis.
'process.env.ALLOW_EXPERIMENT_REREGISTRATION':
process.env.npm_package_config_ALLOW_EXPERIMENT_REREGISTRATION,
// Inject the `IS_WORDPRESS_CORE` global, used for feature flagging.
'process.env.IS_WORDPRESS_CORE':
process.env.npm_package_config_IS_WORDPRESS_CORE,
} ),
mode === 'production' && new ReadableJsAssetsWebpackPlugin(),
];
Expand Down
2 changes: 1 addition & 1 deletion typings/gutenberg-env/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
interface Environment {
NODE_ENV: unknown;
IS_GUTENBERG_PLUGIN?: boolean;
ALLOW_EXPERIMENT_REREGISTRATION?: boolean;
IS_WORDPRESS_CORE?: boolean;
}
interface Process {
env: Environment;
Expand Down

0 comments on commit 09eeb8c

Please sign in to comment.