-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Load FSE php files only if experiment is enabled #24182
Merged
noahtallen
merged 6 commits into
master
from
try/avoid-loading-fse-php-if-experiment-disabled
Aug 7, 2020
Merged
Load FSE php files only if experiment is enabled #24182
noahtallen
merged 6 commits into
master
from
try/avoid-loading-fse-php-if-experiment-disabled
Aug 7, 2020
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
noahtallen
requested review from
epiqueras,
youknowriad,
vindl,
Addison-Stavlo and
Copons
July 24, 2020 03:01
Size Change: 0 B Total Size: 1.15 MB ℹ️ View Unchanged
|
epiqueras
reviewed
Jul 24, 2020
epiqueras
reviewed
Jul 27, 2020
noahtallen
force-pushed
the
try/avoid-loading-fse-php-if-experiment-disabled
branch
from
July 27, 2020 21:59
a9428b2
to
7d7a936
Compare
epiqueras
approved these changes
Jul 28, 2020
I like this, it clarifies what's FSE and what's not. Thanks. |
vindl
approved these changes
Jul 29, 2020
noahtallen
force-pushed
the
try/avoid-loading-fse-php-if-experiment-disabled
branch
from
July 30, 2020 00:29
7d7a936
to
a166a45
Compare
The e2e tests seem to be related to the change. I don't have time to investigate for the next few days, so anyone can feel free to investigate |
noahtallen
force-pushed
the
try/avoid-loading-fse-php-if-experiment-disabled
branch
from
August 5, 2020 23:19
a166a45
to
03c8f60
Compare
- otherwise will cause fatal when FSE experiment is disabled
noahtallen
force-pushed
the
try/avoid-loading-fse-php-if-experiment-disabled
branch
from
August 7, 2020 22:10
f7bf2e5
to
5b7e407
Compare
the e2e failures were due to calling an edit-site function even when the FSE experiment was disabled. |
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.
Description
Previously, any 3rd party who had the Gutenberg plugin installed could use
register_post_type( 'wp_template' )
to opt into block template resolution. Specifically, our template loader file is always loaded and the filter is always registered. When the filter runs, it is gated only by a check for the wp_template CPT. Obviously, anyone can register any CPT they like, so this means that folks can treat template resolution as a stable API simply by registering the CPT themselves.This is motivated by #24129, in which a user had created the wp_template CPT themselves without the experiment enabled, expecting template resolution to work correctly and to be a stable API.
There are some other examples of this (e.g. the edit-site exporter API is always registered if the plugin is installed).
What if we just avoid loading those files altogether if the experiment is disabled? (This is what I would have expected originally.)
My only concern is that this could be considered a breaking change. I know some others think it might not be a breaking change.
How has this been tested?
Locally, in edit site. Tests should pass
Screenshots
Types of changes
Checklist: