#63212 closed enhancement (fixed)
Enhancement: Do not throw PHP notice when file is not a pattern registration file
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | trunk |
Component: | Themes | Keywords: | has-patch changes-requested fixed-major dev-reviewed |
Focuses: | Cc: |
Description
Hi,
Since testing WP6.8 RC1 I get this PHP notice:
Notice: Function get_block_patterns was called incorrectly. Could not register file "./themes/theme-slug/patterns/affected/file.php" as a block pattern ("Slug" field missing) Please see Debugging in WordPress for more information. (This message was added in version 6.0.0.) in ./wp-includes/functions.php on line 6121
Could you please modify the code to skip all PHP files without "Slug" defined in theme-slug/patterns
folder?
As maybe such files are not meant for a pattern registration purpose. They may be helper files for patterns, for example.
That way the notice is not triggered and file is skipped during automatic WordPress pattern registration.
My usecase for explanation:
In my theme I register most of the patterns with proper file metadata.
However, one of the patterns runs dynamic PHP code that has to be registered later on front-end (during wp
action hook) to receive proper data from WordPress. I need to register this pattern using PHP `register_block_pattern()` function instead of setting the pattern file metadata.
To fix the new WP6.8 PHP notice I would need to move the affected pattern from patterns/
folder to other folder, such as patterns-other/
, which creates inconsistency in my theme code organization.
Another alternative solution for me would be to register all patterns with register_block_pattern()
function and not use patterns/
folder at all...
Pre-WP6.8 solution was to have pattern files with metadata as direct children of patterns/
folder and use subfolders for the patterns registered via register_block_pattern()
function.
Proposed solution:
The solution is as simple as removing _doing_it_wrong()
block from if ( empty( $pattern['slug'] ) ) {...}
statement in get_block_patterns()
function (check the code at GitHub).
Change History (25)
#2
@
8 weeks ago
- Summary changed from WP6.8-RC1: Enhancement: Do not throw PHP notice when file is not a pattern registration file to Enhancement: Do not throw PHP notice when file is not a pattern registration file
This ticket was mentioned in PR #8631 on WordPress/wordpress-develop by @webmandesign.
8 weeks ago
#3
- Keywords has-patch added
Skipping non-pattern-definition PHP files within theme-slug/patterns
folder during block pattern registration.
This allows using helper PHP files within theme-slug/patterns
folder, keeping pattern related code in the theme together.
Trac ticket: https://core.trac.wordpress.org/ticket/63212
#4
@
8 weeks ago
@audrasjb Please note that this issue occurs for WP6.8 only.
This is due to the change in WP6.8 when all files including files in subfolders are being used to register patterns within theme-slug/patterns
folder.
Previously only direct children of theme-slug/patterns
folder were used, not the files in subfolders.
The change is related to WP6.8 development, to ticket #62378
#5
@
8 weeks ago
- Milestone changed from Awaiting Review to 6.8
- Version set to trunk
AH. Sorry I misinterpreted the issue. I'm moving it back to 6.8 so we can try to sort it out for RC3. Otherwise it will be moved to 6.8.1. Thank you for clarifying.
#6
@
8 weeks ago
Thank you @audrasjb
I'm hoping the issue is going to be fixed for 6.8 release. The fix is very easy if you consider my proposition.
If the fix is postponed to 6.8.1 or later, I would either need to advice my theme users to wait for such late WordPress update or change the name of the patterns
folder to something else temporarily and then change it back to patterns
for WP6.8.1+ for all of my block themes. That seems to me as an unnecessary walkaround affecting my theme users with crucial, yet somehow unnecessary theme updates.
Thanks for understanding.
#8
follow-up:
↓ 10
@
8 weeks ago
Hi @webmandesign. Could you give me more info about how you previously had set up your file structure for patterns? It sounds like you previously had sub-folders in the /patterns directory that would not get autoloaded, but now they are being autoloaded since [59872] and this causing the warning you describe. Is that correct?
I don't think we will want to remove the _doing_it_wrong()
that you are suggesting, but perhaps we can develop a workaround to support organizing pattern files in your /patterns
directory that should not be autoloaded.
#9
@
8 weeks ago
Hi @joemcgill
Your presumption is correct.
As an example I will use my publicly available theme Zooey that has slightly different usecase than the one I originally described in this ticket:
I have subfolders in patterns/
directory to organize my block patterns and I register these patterns with `register_block_pattern()` function providing user a way to filter the patterns list before registration (this will become a theme option in future update so user can disable patterns they don't use).
So, even though I use patterns/
directory to organize my block patterns, I don't provide file metadata for their registration - I use the register_block_pattern()
function. All of the files in patterns/
directory are throwing the PHP notice in WP6.8.
It almost looks like I can not use register_block_pattern()
function together with patterns/
directory to organize my patterns.
Solution for a theme like Zooey would be as simple as renaming patterns/
directory to pattern/
. But it kind of looks wrong as I have plural directory names in the theme folder structure, such as parts
, templates
and using singular pattern
looks weird to me.
But maybe renaming to block-patterns/
would do the trick, though it feels really weird to me that I have to do this hack.
You can test this theme example in WP playground - check the logs (FYI: there is also _load_textdomain_just_in_time
PHP notice for this particular theme I will fix in the theme update).
I can come up with additional usage examples, though:
Let's say I have this block pattern files structure in my theme:
patterns/ helpers/ icon.php columns/ icons-in-2-columns.php icons-in-3-columns.php
The patterns/helpers/icon.php
is not a pattern file. It is a helper file I use not to repeat the same code across multiple pattern files. This is the content of the file:
<!-- wp:image {"sizeSlug":"full"} --> <figure class="wp-block-image size-full"> <img src="<?php echo esc_url( get_theme_file_uri( 'assets/images/icon-demo.webp' ) ); ?>" alt="Demo icon"/> </figure> <!-- /wp:image -->
Then I include the patterns/helpers/icon.php
in the actual pattern files within patterns/columns/
folder.
With this example I'd get PHP notice for the patterns/helpers/icon.php
file.
I'd like to keep the patterns/helpers/icon.php
in the patterns/
folder because I use it only for patterns, so I like to keep patterns code organized together.
#10
in reply to:
↑ 8
@
8 weeks ago
Replying to joemcgill:
I don't think we will want to remove the
_doing_it_wrong()
that you are suggesting, but perhaps we can develop a workaround to support organizing pattern files in your/patterns
directory that should not be autoloaded.
Well, that's pity :)
In that case I would take some filter hook too. Such as providing a filter for `$files` variable.
(Or possibly even filtering for related `scandir()` method output, or its $path
variable value, so I can short circuit it.)
This ticket was mentioned in Slack in #core-themes by joemcgill. View the logs.
8 weeks ago
#12
@
7 weeks ago
Hi
I agree that the notice needs to be kept to help with pattern registration.
I understand its an inconvenience, but as I see it there are two options, disable notices or move the files.
#13
follow-up:
↓ 14
@
7 weeks ago
Even if the code was updated to exclude one named folder inside patterns (example: helpers), theme developers that have used subfolders this way would still need to take action, so it would not really solve anything.
#14
in reply to:
↑ 13
@
7 weeks ago
Replying to poena:
Even if the code was updated to exclude one named folder inside patterns (example: helpers), theme developers that have used subfolders this way would still need to take action, so it would not really solve anything.
I agree. Having a special subfolder name to skip is not a valid option even for me. I'd much prefer something more flexible.
I've updated my GitHub PR code and proposing:
- Adding
get_block_patterns_files
(name is subject to change) filter to allow theme authors filter out the array of files withinpatterns
directory before they are processed. This way a theme author doing something non-standard with patterns can manage, while still keeping the defaultpatterns
directory functionality intact.
- I've also added a check whether the pattern file has both slug and title set. If both are empty, then maybe we can assume this is not a pattern file? Surely, this can be removed if this is "too much".
#15
@
7 weeks ago
I agree that pattern registration should be flexible, and the filter-out approach seems logical. This allows developers to exclude non-pattern files from the patterns directory as needed.
However, I believe the core function should throw an error if a pattern file is missing the required slug
metadata. If a developer wants to suppress the error, they can simply filter out the file using the available filter.
#16
@
7 weeks ago
I've forgotten to mention that I returned back the _doing_it_wrong()
PHP notice for slug in my GitHub PR.
So, really the question now is whether we get the filter hook for the pattern files in WP6.8.
#17
@
7 weeks ago
- Keywords changes-requested added
- Status changed from reviewing to accepted
Thanks @webmandesign. I've left a few small bits of feedback on the PR, @poena do you have any concerns with the latest proposal?
#20
@
7 weeks ago
- Keywords dev-feedback fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening for consideration in the 6.8 branch
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
7 weeks ago
#22
@
7 weeks ago
- Keywords dev-reviewed added; dev-feedback removed
The new filter looks good for a 6.8 backport @joemcgill
I'm removing
trunk
version since this wasn't introduced in 6.8 and it is not related to any change that happened in 6.8.