Implement pattern overrides behind IS_GUTENBERG_PLUGIN flag
#59702
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talldan
added
[Type] Task
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/load.php ❔ lib/compat/wordpress-6.6/block-bindings/pattern-overrides.php |
|
Size Change: +124 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
49 tasks
youknowriad
reviewed
Mar 8, 2024
| @@ -284,6 +286,9 @@ export default function ReusableBlockEdit( { | |||
| onNavigateToEntityRecord: | |||
| getSettings().onNavigateToEntityRecord, | |||
| editingMode: _getBlockEditingMode( patternClientId ), | |||
| hasPatternOverridesSource: !! getBlockBindingsSource( | |||
There was a problem hiding this comment.
I wonder why you chose to use the selector over process.env.IS_GUTENBERG_PLUGIN
There was a problem hiding this comment.
Yeah, I was in two minds about it.
My thinking is that this could be maintained to work like this even after the process.env.IS_GUTENBERG_PLUGIN code is removed, as a potential unregisterBindingSource( 'core/pattern-overrides' ) call could result in the same thing.
Actually, I don't think this approach is a good idea, as a WP 6.5 user could re-register pattern overrides, so I think we'll need to add process.env.IS_GUTENBERG_PLUGIN everywhere.
edit: The register action is private at the moment, so maybe this solution is ok. I don't mind implementing it either way though.
There was a problem hiding this comment.
The register action is private at the moment, so maybe this solution is ok.
Given that it's still possible, shall we just put behind the flag?
There was a problem hiding this comment.
@getdave It's not possible for third-parties, so it doesn't impact.
Personally, I don't mind either way.
There was a problem hiding this comment.
Oh I thought a third party could opt into Private APIs it's just they have to jump through a lot of hoops to do so.
Personally, I don't mind either way.
Good enough for me. @talldan can make a call.
getdave
reviewed
Mar 11, 2024
There was a problem hiding this comment.
I tested this by applying the following patch to simulate not being in the Gutenberg Plugin:
Disable Gutenberg Patch
diff --git a/lib/load.php b/lib/load.php
index dc3a6533a5..7d62ce342a 100644
--- a/lib/load.php
+++ b/lib/load.php
@@ -9,7 +9,7 @@ if ( ! defined( 'ABSPATH' ) ) {
die( 'Silence is golden.' );
}
-define( 'IS_GUTENBERG_PLUGIN', true );
+define( 'IS_GUTENBERG_PLUGIN', false );
require_once __DIR__ . '/init.php';
require_once __DIR__ . '/upgrade.php';
diff --git a/tools/webpack/shared.js b/tools/webpack/shared.js
index debd3fc93f..fc348dc7b7 100644
--- a/tools/webpack/shared.js
+++ b/tools/webpack/shared.js
@@ -64,8 +64,7 @@ const plugins = [
process.env.WP_BUNDLE_ANALYZER && new BundleAnalyzerPlugin(),
new DefinePlugin( {
// Inject the `IS_GUTENBERG_PLUGIN` global, used for feature flagging.
- 'process.env.IS_GUTENBERG_PLUGIN':
- process.env.npm_package_config_IS_GUTENBERG_PLUGIN,
+ 'process.env.IS_GUTENBERG_PLUGIN': false,
// Inject the `IS_WORDPRESS_CORE` global, used for feature flagging.
'process.env.IS_WORDPRESS_CORE':
process.env.npm_package_config_IS_WORDPRESS_CORE,I saw that the overrides feature was completely disabled in the UI.
I also tried registering some overrides whilst in the Gutenberg Plugin and then switching back to the "Disabled Gutenberg Plugin" environment and I didn't see anything persisted. All the bindings in the source appeared to have been removed as well.
If there's anything else I could test such as APIs that were previously exposed that would be useful to know. Otherwise this appears to be looking good.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
youknowriad
approved these changes
Mar 11, 2024
youknowriad
added
the
Backport to WP 6.7 Beta/RC
getdave
added a commit
that referenced
this pull request
Mar 11, 2024
Co-authored-by: talldan <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: getdave <[email protected]>
|
I just cherry-picked this PR to the pick/wp-65-rc-2 branch to get it included in the next release: dee8699 |
getdave
added
Backported to WP Core
getdave
added a commit
that referenced
this pull request
Mar 12, 2024
Co-authored-by: talldan <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: getdave <[email protected]>
bph
added
[Type] Bug
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…ess#59702) Co-authored-by: talldan <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: getdave <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
See #53705 (comment).
Puts the pattern overrides feature behind the
IS_GUTENBERG_PLUGINflag to prevent it shipping to core.How?
I've taken the approach of using
IS_GUTENBERG_PLUGINwhere thecore/pattern-overridesblock bindings source is registered.In other places in the codebase, we check for the existence of that
core/pattern-overridesblock binding source to disable the feature.Testing Instructions
This needs some thorough testing to ensure all traces of pattern overrides are no longer present.
To replicate
IS_GUTENBERG_PLUGINbeingfalseyou can do the following:webpack/shared.jsfile, changing this line to'process.env.IS_GUTENBERG_PLUGIN': false. You can usually also modify this in the project package.json itself, but for me this caused a build failure.gutenberg/tools/webpack/shared.js
Lines 67 to 68 in ed7edbc
lib/load.phpfile, changing this line todefine( 'IS_GUTENBERG_PLUGIN', false );:gutenberg/lib/load.php
Line 12 in ed7edbc
Then finally rebuild.