Make WordPress Core

Opened 16 months ago

Last modified 8 weeks ago

#57647 reviewing defect (bug)

Deprecate wp_enqueue_block_support_styles function

Reported by: andrewserong's profile andrewserong Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 6.2
Component: Editor Keywords: has-patch gutenberg-merge changes-requested
Focuses: Cc:

Description

Prior to the style engine classes being added to core, the code for block supports used the wp_enqueue_block_support_styles function to output a <style> tag containing the CSS styles for the block support. Using this function resulted in duplicate <style> tags being output on the site frontend, and redundant CSS rules.

As of 6.2, it looks like all of the block supports and block's individual PHP rendering will have been refactored to use the style engine's approach of enqueuing styles, where styles are registered using a common call to the style engine, and then only output once in a single style tag. For example, the Layout block support uses wp_style_engine_get_stylesheet_from_css_rules instead of wp_enqueue_block_support_styles.

Therefore, it should now be possible to deprecate usage of the wp_enqueue_block_support_styles function, and nudge any usage of that function over to use comparable style engine functions instead.

Note: this deprecation should only occur once all usage of wp_enqueue_block_support_styles has been removed in core blocks, which should happen once the JS packages update for 6.2 has occurred, as that update should replace the call with a call to the style engine instead.

Change History (12)

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


16 months ago
#1

  • Keywords has-patch added

In 6.2, once the JS packages have been updated, which includes changes to core blocks' index.php files, all usage in core of wp_enqueue_block_support_styles will have been removed. It should then be possible to deprecate wp_enqueue_block_support_styles as a function.

Since the style engine classes were added in 6.1, the style engine is now a better way to enqueue block support styles, as styles can be registered in multiple places, with those styles grouped together to be output only a single time, whereas wp_enqueue_block_support_styles resulted in multiple <style> tags being output on the site frontend (one for each instance of a block support being rendered), along with redundant style output.

Note: this PR should only land after the JS packages update that includes the changes to the gallery block, which roll in this PR: https://github.com/WordPress/gutenberg/pull/43070. Without that, then you if a request a post or page containing a Gallery block, the following will be logged, as the gallery block in trunk currently still contains a call to wp_enqueue_block_support_styles:

https://i0.wp.com/user-images.githubusercontent.com/14988353/217120189-616a63a3-11e3-4fcf-831b-ee92eae282e6.png

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

CC: @Mamaduka

@andrewserong commented on PR #4015:


16 months ago
#2

Note: the failing PHP tests are expected here! Once the JS packages update has been completed, this PR should be rebased against trunk and the following test failures should no longer occur:

1) Tests_Blocks_Render::test_do_block_output with data set #21 ('core__gallery.html', 'core__gallery.server.html')
Unexpected deprecation notice for wp_enqueue_block_support_styles.
Function wp_enqueue_block_support_styles is deprecated since version 6.2.0! Use wp_style_engine_get_stylesheet_from_css_rules() instead.
Failed asserting that an array is empty.

2) Tests_Blocks_Render::test_do_block_output with data set #22 ('core__gallery__columns.html', 'core__gallery__columns.server.html')
Unexpected deprecation notice for wp_enqueue_block_support_styles.
Function wp_enqueue_block_support_styles is deprecated since version 6.2.0! Use wp_style_engine_get_stylesheet_from_css_rules() instead.
Failed asserting that an array is empty.

#3 @andrewserong
16 months ago

  • Keywords gutenberg-merge added

@Mamaduka commented on PR #4015:


16 months ago
#4

Thank you, @andrewserong!

The PR will need to be rebased after #3914 is merged to fix PHPUnit tests.

#5 @Mamaduka
16 months ago

  • Milestone changed from Awaiting Review to 6.2

@andrewserong commented on PR #4015:


16 months ago
#6

Update: even thought the tests are now passing, it looks like this PR is likely not going to be viable in time as a recently backported feature (#4013) appears to also use wp_enqueue_block_support_styles. From my perspective, there's no urgency in deprecating the feature, this PR can happily wait (or be parked) until there's no longer any calls to the function. Or, alternately, it's okay to keep the function around as a not-deprecated function if there's a valid use for the existing behaviour.

@Mamaduka commented on PR #4015:


16 months ago
#7

Thank you, @andrewserong!

#8 @hellofromTonya
16 months ago

  • Keywords changes-requested added
  • Owner set to hellofromTonya
  • Status changed from new to reviewing

Self-assigning for review and commit (when ready).

Marking for changes requested.

#9 @hellofromTonya
16 months ago

Checking for usages of wp_enqueue_block_support_styles() in wp.org's plugin and theme directories:

  • Plugins: 2 plugins affected each with <= 10 active installations

Query: https://wpdirectory.net/search/01GS69QYQQF9842PKYDV2FZR71
The first 2 in the query results are false positives.

  • Themes: 0 are affected

Query: https://wpdirectory.net/search/01GS6APD4W9YZ04HY8VG1FCD60

#10 @hellofromTonya
16 months ago

  • Milestone changed from 6.2 to Future Release

Note: This ticket is blocked due to [55255] added _wp_add_block_level_preset_styles() that invokes wp_enqueue_block_support_styles().

For this ticket to move forward, first the _wp_add_block_level_preset_styles() needs:

  • to be analyzed to determine if wp_style_engine_get_stylesheet_from_css_rules() can be used instead of wp_enqueue_block_support_styles().
  • if yes, then the change needs to happen in Gutenberg, go through a Gutenberg release, and then backported to Core.

As there's likely not time for this to happen in 6.2, I'll move this ticket to Future Release. Once _wp_add_block_level_preset_styles() is updated, then this ticket can be moved into a milestone.

@andrewserong commented on PR #4015:


16 months ago
#11

Thanks for reviewing @hellofromtonya — I think this PR is stalled right now due to https://github.com/WordPress/wordpress-develop/pull/4013 landing after I opened this PR. I added a comment to that PR with some feedback about possible next steps: https://github.com/WordPress/wordpress-develop/pull/4013#issuecomment-1421566828.

Given the timing, and that I don't think refactoring that usage would be very straightforward, I think we should probably park this deprecation proposal for the time being. I can close out this PR if it keeps things neater 🙂

#12 @hellofromTonya
8 weeks ago

  • Owner hellofromTonya deleted

As of today, I'm no longer a sponsored contributor and am starting a month or more AFK break. To not stand in the way, I'm clearing me as the ticket owner, which opens the ticket for someone to step in to shepherd this ticket forward to resolution.

Note: See TracTickets for help on using tickets.