-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conditionally load PHP classes #10531
Conversation
These will soon land in WordPress core, and we don't want PHP to fatal by trying to load them twice.
@WordPress/gutenberg-core I'd like to consider this for inclusion in Gutenberg 4.0 because sooner is better than later. However, it's not critical to have in 4.0, so I'd respect any strong opinions against. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes a whole lot of sense to me, and I think worth adding to 4.0.
Gets the 👍 from me to include in 4.0 as well, I suppose in 4.0.0-rc.2
.
I'd say if there isn't any opposition by tomorrow afternoon UTC we should put this in the 4.0 milestone, roll another RC with this included, and update the changelog(s). |
I'd say this solves the issue partially (functions could be duplicates, registered styles and scripts as well). What about this alternative plan https://wordpress.slack.com/archives/C02QB2JS7/p1539288501000100 ? Anyone able to help with that? |
@mtias said Wednesday / Thursday timing for 4.1, so I don't think it's critical to ship in 4.0.
Yes, that needs to be addressed to. Should we open a new issue to sort those specifics out? iirc some functions are being renamed, so that's less of an issue. |
Oh, gotcha then! Cool, ignore me 😄 |
require dirname( __FILE__ ) . '/class-wp-rest-search-controller.php'; | ||
require dirname( __FILE__ ) . '/class-wp-rest-search-handler.php'; | ||
require dirname( __FILE__ ) . '/class-wp-rest-post-search-handler.php'; | ||
if ( ! class_exists( 'WP_REST_Blocks_Controller' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if there's a reason why we use these class checks, rather than require_once
for every class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kwight require_once
works based on file path, not the class defined within the file. Because WordPress core's paths are different, you'd end up with fatals if you used require_once
.
These will soon land in WordPress core, and we don't want PHP to fatal by trying to load them twice.
Closes #10526.