Update the Patterns API to avoid ambiguity #21970
Conversation
youknowriad
added
[Type] New API
youknowriad
requested review from
ajitbohra,
chrisvanpatten,
mkaz and
TimothyBJacobs
as code owners
|
Size Change: 0 B Total Size: 816 kB ℹ️ View Unchanged
|
| * | ||
| * @package Gutenberg | ||
| */ | ||
|
|
||
| /** | ||
| * Class used for interacting with patterns. | ||
| */ | ||
| final class WP_Patterns_Registry { | ||
| final class WP_Block_Patterns_Registry { |
There was a problem hiding this comment.
Not directly related to the renaming, but I wonder here about pluralization, specifically since there's already precedent in core with using the singular form for a "registry of many X", i.e. WP_Block_Type_Registry. Should these not be consistent? (WP_Block_Pattern_Registry)
Edit: After this, I realize there is WP_Block_Styles_Registry as well, so inconsistency already exists unfortunately 😞
There was a problem hiding this comment.
Do you have a preference knowing the inconsistency exists?
There was a problem hiding this comment.
Looking through some of the other core classes for prior art, I do also see there's another example of the plural form with WP_Dependencies (WP_Scripts, WP_Styles), which is another sort of "registry of items". Maybe it's best to lean into this, and consider WP_Block_Type_Registry as the exception.
| * | ||
| * @return WP_Block_Patterns_Registry The main instance. | ||
| */ | ||
| public static function get_instance() { |
There was a problem hiding this comment.
Technically one could have created their own WP_Patterns_Registry via new WP_Patterns_Registry. In fact, if we had any unit tests for this, I assume it would have been done this way. We don't currently deprecate any of the instance members, so custom constructed objects won't proxy through to the new class. I guess we could choose between: (a) Consider it unlikely and accept it, (b) add deprecations for all instance members.
There was a problem hiding this comment.
(a) Consider it unlikely and accept it
I'm leaning towards this because it's useless since it won't be even passed to the editor?
| * @return WP_Block_Patterns_Registry The main instance. | ||
| */ | ||
| public static function get_instance() { | ||
| _deprecated_function( 'WP_Patterns_Registry', 'Gutenberg 8.3.0', 'WP_Block_Patterns_Registry' ); |
There was a problem hiding this comment.
I missed two things:
- Should this be 8.1.0 ? (The version at which it's deprecated, not the version at which it's removed?)
- We should add a section for the removal in 8.3.0 in the deprecations document
closes #21655
This PR renames the Block Patterns API following the discussion here.