WordPress / theme-check

Theme Check plugin
https://wordpress.org/plugins/theme-check/
341 stars 114 forks source link

Throw an error if a theme erroneously uses the full-site-editing tag #428

Closed dd32 closed 1 year ago

dd32 commented 2 years ago

Some themes may incorrectly add the full-site-editing tag, either accidentally not understanding it's purpose, or inappropriately to appear in the new Block Themes tab

The usage of this tag should be limited to themes which are block themes, so we should throw an error from Theme Check to deny a theme update that adds that tag to a non-block-theme.

kafleg commented 2 years ago

Thank you for making the issue. We need to write the code and update the plugin.

justintadlock commented 2 years ago

There is already a FSE required files check: https://github.com/WordPress/theme-check/blob/master/checks/class-fse-required-files-check.php

However, it is only run after TC has already determined that it is a block theme: https://github.com/WordPress/theme-check/blob/master/checkbase.php#L353-L356

If it's not a block theme, it bails early:

// Check whether this is a FSE theme by searching for an index.html block template.
if ( ! in_array( 'block-templates/index.html', $other_filenames, true ) && ! in_array( 'templates/index.html', $other_filenames, true ) ) {
    return false;
}

I think we could probably adjust that conditional to also run the FSE required files check if the theme has the full-site-editing tag.

dd32 commented 2 years ago

While we could target it to that specific code branch, I'm pondering if it would make sense to add a Tag_Check_Requirements check which for defined tags, checks the appropriate requirements for that tag?

In other words, right now it's mostly a FSE-tag thing, but, maybe we could in the future add checks for a theme actually supporting block-styles or editor-style (and doing so, the proper way, by calling the correct functions, rather than some hacky injection, or not supporting it at all).

Perhaps this can wait for being v2 of the check though, and for now, we can just replace that return false for a if has tag, throw error and return false, else just return false.

kafleg commented 1 year ago

Created PR and checked both parent theme and child theme. Please review this.