From 7e9d9449010593d54ca72dee1fbd4beb4e38ec80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 23 Feb 2023 11:50:27 +0100 Subject: [PATCH 1/4] [Private APIs] Make the re-registration safeguard opt-in rather than opt-out 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 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: https://github.com/WordPress/wordpress-develop/pull/4121 --- packages/private-apis/src/implementation.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/private-apis/src/implementation.js b/packages/private-apis/src/implementation.js index 6a88e9812ca8c0..249435312f4c95 100644 --- a/packages/private-apis/src/implementation.js +++ b/packages/private-apis/src/implementation.js @@ -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 ALLOW_EXPERIMENT_REREGISTRATION +// is set to false. +// 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.ALLOW_EXPERIMENT_REREGISTRATION ?? true; } catch ( error ) { - allowReRegistration = false; + allowReRegistration = true; } /** From 6121339ea8223b26d245d4f6ac5dc7a1cd69ff62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 23 Feb 2023 12:56:02 +0100 Subject: [PATCH 2/4] Remove defaults for ALLOW_EXPERIMENT_REREGISTRATION in other config files --- package.json | 3 +-- storybook/main.js | 6 ------ test/native/setup.js | 8 -------- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/package.json b/package.json index 5b5cff76529dae..955616b8b6be30 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/storybook/main.js b/storybook/main.js index dccaf726381308..a0bd17068a60e4 100644 --- a/storybook/main.js +++ b/storybook/main.js @@ -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, - } ), }; diff --git a/test/native/setup.js b/test/native/setup.js index b9e32a51d28799..5f1801a338985f 100644 --- a/test/native/setup.js +++ b/test/native/setup.js @@ -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' ); From f711c5f363c0c3bd224fd516ced72dd0f84184ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Tue, 28 Feb 2023 11:47:04 +0100 Subject: [PATCH 3/4] Replace ALLOW_EXPERIMENT_REREGISTRATION with IS_WORDPRESS_CORE --- packages/private-apis/src/implementation.js | 6 +++--- typings/gutenberg-env/index.d.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/private-apis/src/implementation.js b/packages/private-apis/src/implementation.js index 249435312f4c95..c37ae7b227bc9e 100644 --- a/packages/private-apis/src/implementation.js +++ b/packages/private-apis/src/implementation.js @@ -49,13 +49,13 @@ const requiredConsent = /** @type {boolean} */ let allowReRegistration; -// The safety measure is meant for WordPress core where ALLOW_EXPERIMENT_REREGISTRATION -// is set to false. +// 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 ?? true; + allowReRegistration = process.env.IS_WORDPRESS_CORE ? false : true; } catch ( error ) { allowReRegistration = true; } diff --git a/typings/gutenberg-env/index.d.ts b/typings/gutenberg-env/index.d.ts index 834c307515893b..3b9856b587616c 100644 --- a/typings/gutenberg-env/index.d.ts +++ b/typings/gutenberg-env/index.d.ts @@ -1,6 +1,7 @@ interface Environment { NODE_ENV: unknown; IS_GUTENBERG_PLUGIN?: boolean; + IS_WORDPRESS_CORE?: boolean; ALLOW_EXPERIMENT_REREGISTRATION?: boolean; } interface Process { From a00f3f14a27bdd714dcf8638f62d85e55f0e7a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Tue, 28 Feb 2023 11:57:40 +0100 Subject: [PATCH 4/4] Refactor the remaining uses of ALLOW_EXPERIMENT_REREGISTRATION --- test/unit/config/gutenberg-env.js | 10 +++++----- tools/webpack/shared.js | 6 +++--- typings/gutenberg-env/index.d.ts | 1 - 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/test/unit/config/gutenberg-env.js b/test/unit/config/gutenberg-env.js index 72527ecb725a3c..a6a1b81f740970 100644 --- a/test/unit/config/gutenberg-env.js +++ b/test/unit/config/gutenberg-env.js @@ -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, }; diff --git a/tools/webpack/shared.js b/tools/webpack/shared.js index 31a9245f7e35ea..9aaa35737400c9 100644 --- a/tools/webpack/shared.js +++ b/tools/webpack/shared.js @@ -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(), ]; diff --git a/typings/gutenberg-env/index.d.ts b/typings/gutenberg-env/index.d.ts index 3b9856b587616c..e2876716bd8b7f 100644 --- a/typings/gutenberg-env/index.d.ts +++ b/typings/gutenberg-env/index.d.ts @@ -2,7 +2,6 @@ interface Environment { NODE_ENV: unknown; IS_GUTENBERG_PLUGIN?: boolean; IS_WORDPRESS_CORE?: boolean; - ALLOW_EXPERIMENT_REREGISTRATION?: boolean; } interface Process { env: Environment;