Make WordPress Core

Opened 5 months ago

Last modified 4 months ago

#61958 assigned enhancement

Rename `wp_should_load_separate_core_block_assets()` to clarify its purpose

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Editor Keywords: 2nd-opinion has-patch
Focuses: template Cc:

Description

The wp_should_load_separate_core_block_assets() function and the should_load_separate_core_block_assets filter were introduced in WordPress 5.8 (see [50836] and dev note).

While initially the function's and filter's purpose was to determine whether to load individual block stylesheets for core blocks (vs the one large wp-block-library stylesheet), this purpose has notably changed over time: Today, this function is called to determine whether or not to load block assets on demand (vs unconditionally throughout the entire site). In other words, it also covers third-party blocks, but the entire purpose is different. It still also controls whether to use individual stylesheets for the core blocks, but at this point that is mostly a side effect of loading block assets on demand.

The function name, filter name, and their documentation is confusing. Not only does it not explain what it actually does today, it is also inconsistent in itself in that it sometimes refers to "core block types", while other times it refers to "block types" (in general). This Slack thread is an example of how the current naming and documentation leads to confusion.

This ticket proposes renaming the function and filter and updating the documentation to clarify the purpose. Of course, technically we can't just rename things, so this will mean deprecating the current function and filter in favor of a new function and filter.

Potentially a good name would be wp_should_load_block_assets_on_demand. This could work for both the function and the filter. The documentation should then focus on this primary purpose, while additionally explaining that this will furthermore lead to individual core blocks stylesheets being used instead of wp-block-library (which still makes sense in this context, because loading wp-block-library is anything but on demand 🙃).

For additional context, see this Slack thread, which is based on the other thread above.

Change History (10)

#1 @flixos90
5 months ago

@gziolo @aristath Based on your involvement in the original function back then, it would be great to get your perspective.

@fabiankaegy Your thread led to opening this ticket, so your feedback would be appreciated too.

This ticket was mentioned in Slack in #core-editor by flixos90. View the logs.


5 months ago

#3 follow-up: @swissspidy
5 months ago

IMHO there isn't much value in renaming this function, it will just cause unnecessary work for extenders and maintenance burden for core maintainers. We'd just need to keep back compat for 2 functions and filters.

+1 to improving the documentation though. Maybe we can just start with that and go from there?

#4 in reply to: ↑ 3 @flixos90
5 months ago

Replying to swissspidy:

IMHO there isn't much value in renaming this function, it will just cause unnecessary work for extenders and maintenance burden for core maintainers.

I disagree. The function and filter names are confusing (as proven by the Slack thread for example), so there is value in renaming them. There would be an easy migration path, and deprecations will realistically never be removed anyway, so there's no breakage. Code using the old function/filter would continue to work forever, they'd just be encouraged to migrate.

#5 @aristath
5 months ago

I think renaming these makes sense...
When the function and the filter were first introduced, their scope was narrower, and the names made sense.
That is no longer the case, and they are the cause of some confusion.

+1 from me.

#6 @flixos90
5 months ago

Related is #61965, which proposes having a way to do what wp_should_load_separate_core_block_assets() appears to do today (based on its name), independently of what wp_should_load_separate_core_block_assets() actually does today. 🤯

#7 follow-up: @desrosj
4 months ago

@flixos90 are you still looking to get this committed for 6.7? The beta 1 feature deadline is in ~22 hours.

#8 in reply to: ↑ 7 @flixos90
4 months ago

Replying to desrosj:

@flixos90 are you still looking to get this committed for 6.7? The beta 1 feature deadline is in ~22 hours.

I'll go over the tickets assigned to me tomorrow and will either commit or punt them.

#9 @flixos90
4 months ago

  • Milestone changed from 6.7 to 6.8

Punting this to 6.8, as a renaming of a function also relevant to Gutenberg should happen earlier in the cycle.

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


4 months ago
#10

  • Keywords has-patch added

Trac ticket: Core-61958

## Summary

  • This pull request addresses the confusion surrounding the wp_should_load_separate_core_block_assets() function and the should_load_separate_core_block_assets filter, both introduced in WordPress 5.8. The goal is to clarify their purpose and improve the documentation.

## Background

  • Initially, the wp_should_load_separate_core_block_assets() function and its associated filter were intended to determine whether individual block stylesheets for core blocks should be loaded, as opposed to the single wp-block-library stylesheet. However, their purpose has evolved to control the loading of block assets on demand, including for third-party blocks. This change has led to inconsistencies in naming and documentation, which have caused confusion among developers.

## Key Changes

### Function and Filter Renaming

  • The old function wp_should_load_separate_core_block_assets() has been deprecated and replaced with wp_should_load_block_assets_on_demand().
  • The associated filter has also been renamed accordingly.

### Deprecation Implementation:

  • The deprecated function has been moved to deprecated.php to maintain backward compatibility while encouraging developers to transition to the new function.

### Codebase Updates:

  • All instances of the old function have been replaced with the new function and filter throughout the core codebase.
  • Documentation Updates: The documentation for the new function and filter has been updated to clearly outline their purpose, focusing on the loading of block assets on demand and the implications for core block stylesheets.

## Benefits:

  • Enhanced clarity and consistency in the naming and documentation of functions and filters.
  • Improved performance by allowing for conditional loading of block assets.
  • A smoother transition for developers by maintaining backward compatibility through deprecation.
Note: See TracTickets for help on using tickets.