Handle post type support in `WP_Post_Type`

[flixos90]

Currently post type features are stored in a global variable $_wp_post_type_features. Now that we have WP_Post_Type, I think it would make sense to let the class handle this functionality. Post type features are part of a post type, so they should also be part of it in the implementation. This would also allow us to get rid of the global variable we currently use (note that it's private, so it's not intended to be used directly by other developers).

There's a single possible caveat I would see with this implementation: That would be if post type support is added before the post type is even registered. However, I would think we might be able to change this (with a dev note) since it should be best practices to add post type support either directly (via the supports argument in register_post_type()) or after registering the post type. Adding a post type feature without the post type being registered is not logical in my opinion.

[flixos90]

37667.diff​ is a patch for what the implementation of this proposal could look like.

All features are stored in a private $features property in WP_Post_Type. The related methods explicitly handle subtypes (like we have for attachments with attachment:audio and attachment:video) - for example, calling $attachment_post_type->supports( 'thumbnail', 'audio' ) would be the same as calling post_type_supports( 'attachment:audio', 'thumbnail' ).

There's a method remove_supports() in WP_Post_Type which might not be necessary any longer after this change. It could be left as is or deprecated.

[swissspidy]

+1 for pursuing this.

[flixos90]

@swissspidy: What do you think about the (possible) problem with adding post type support before the post type is registered? How should we handle this? Add bool return values to add_post_type_support()? Or return WP_Error? Or _doing_it_wrong()?

[swissspidy]

@flixos90 wp_oembed_add_provider() and WP_oEmbed::_add_provider_early() come to mind.

If the post type doesn't exist yet, the feature could still be added to the global and upon post type registration it would be taken out of there and added to the actual post type object.

Adding a return value wouldn't help much as developers don't check them since it's a void function. A _doing_it_wrong() could be triggered so that eventually we might get rid of the global array in the future.

[flixos90]

37667.2.diff​ is an updated patch.

I checked how WP_oEmbed handles their almost similar problem, however what's different is that we are likely to remove that backwards-compatible functionality at some point (to get rid of the global). We could have gotten rid of the global now by moving the functionality to a static property and some (semi-)privat static methods, but I don't think we should add a bunch of functionality there if we're going to get rid of it at some point. I added _doing_it_wrong() notices that are thrown when calling add_post_type_support() or remove_post_type_support() before the post type is registered. It still works now, using the global, but that should be removed later as discussed. The other functions get_all_post_type_supports(), post_type_supports() and get_post_types_by_support() don't throw a notice as I think they can be used before the post types are registered. They will just return an empty array (or false) when we get rid of the global.

I also added several unit tests for the new methods of WP_Post_Type and the post type support functions.

[flixos90]

37667.3.diff​ actually gets rid of the $_wp_post_type_features global by using a similar technique like WP_oEmbed does (early post type features).

[desrosj]

37667.3.diff​ needs a refresh to apply cleanly.