Make WordPress Core

Opened 14 months ago

Closed 12 months ago

Last modified 12 months ago

#57736 closed enhancement (fixed)

Remove unused parameter in query for `get_block_templates` method

Reported by: oandregal's profile oandregal Owned by: oandregal's profile oandregal
Milestone: 6.3 Priority: normal
Severity: normal Version: 5.8
Component: Themes Keywords: has-patch commit
Focuses: template Cc:

Description

I've run into a piece of code that calls the get_block_templates function.

This is it:

<?php
$query     = array(
    'theme'    => get_stylesheet(),
    'slug__in' => $slugs,
);
$templates = get_block_templates( $query );

While querying by the active theme looks sensible, that's not how get_block_templates implementation works: the theme parameter is not used anywhere, so it can be safely removed to avoid inducing the reader it is an allowed filter to get_block_templates.

Change History (11)

This ticket was mentioned in PR #4086 on WordPress/wordpress-develop by @oandregal.


14 months ago
#1

  • Keywords has-patch added

Trac ticket https://core.trac.wordpress.org/ticket/57736

This PR removes the theme parameter from the query passed to get_block_templates. It is not used or documented. This removal doesn't have any effect in the code, as the data is ignored anyway.

#3 @davidbaumwald
14 months ago

@oandregal Would this be a bug? If so, I think we can add it to 6.2 and merge this quickly.

#4 @oandregal
14 months ago

I don't see anything that misbehaves due to this, so I don't think it's a bug right now, no.

However, it is still wrong, and it could induce readers of this code to use this pattern in other parts of the codebase thinking get_block_templates admits that filter and using it inappropriately.

Hope this helps.

#5 @davidbaumwald
14 months ago

  • Milestone changed from Awaiting Review to 6.3
  • Version changed from trunk to 5.8

@oandregal Thanks. Yeah it seems to not break anything, but it's dead code and - however small - it's dead code that's running. Added in [51003] in 5.8.

Asking for @hellofromTonya's opinion. Otherwise, this can certainly go in 6.3.

#6 @hellofromTonya
14 months ago

Thanks @davidbaumwald and @oandregal!

I agree with @davidbaumwald for it to land in 6.3, rather than 6.2. Why?

  • It's not a gutenberg-merge.
  • It's an enhancement, i.e. 6.2 is past the enhancement window.
  • Wasn't introduced in the 6.2 cycle.
  • It's not breaking anything.

Once 6.2 is branched (at RC), then this can be committed. I'm all for removing non-BC dead code. Good find @oandregal!

#7 @oandregal
14 months ago

Oh, now I see. I didn't remember we were at "only bug fixes for trunk". Absolutely, this can land after 6.2 is branched out. Thanks for clarifying my confusion.

#8 @audrasjb
13 months ago

  • Keywords commit added

I performed a quick search and it appears that no known plugin use this parameter. Which sounds logical as it was never documented anywhere, but worth checking.

marking this as ready for commit.

#9 @oandregal
12 months ago

  • Owner set to oandregal
  • Status changed from new to assigned

#10 @oandregal
12 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 55686:

Themes: remove unused parameter in query for get_block_templates().

Remove the theme parameter from the query passed to get_block_templates(). It is not used or documented. This removal doesn't have any effect in the code, as the data is ignored anyway.

Props draganescu, audrasjb, davidbaumwald, hellofromTonya.
Fixes #57736.

Note: See TracTickets for help on using tickets.