Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#49387 closed defect (bug) (fixed)

Run pre_render_block filter in block-renderer REST controller

Reported by: kadamwhite's profile kadamwhite Owned by: kadamwhite's profile kadamwhite
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.1
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: rest-api Cc:

Description

Since 5.1, the render_block method in wp-includes/blocks.php calls the pre_render_block filter and short-circuits block rendering if that filter returns a value. This can be used to implement alternative block rendering systems, but the filter is not called when rendering a block via the /block-renderer API endpoints. This leads the rendered preview of the blocks in the editor to differ from the frontend if these filters are in use.

I discussed this briefly with @aduth the #core-editor slack channel, and we agreed it seemed like an unintentional omission. If code on the site is going to change the output of a block on the frontend, that output should be visible in the ServerSideRender component's output.

Attachments (2)

49387.1.diff (2.8 KB) - added by kadamwhite 5 years ago.
pass block attributes through pre_render_block filter in block-renderer endpoint
49387.2.diff (2.9 KB) - added by kadamwhite 5 years ago.
Delegate to render_block within block-renderer endpoint

Download all attachments as: .zip

Change History (16)

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


5 years ago

@kadamwhite
5 years ago

pass block attributes through pre_render_block filter in block-renderer endpoint

#2 @kadamwhite
5 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

@TimothyBlynJacobs (and possibly @matveb), This is a small patch that irons out a frustration I've had lately with the block renderer endpoint. If you have bandwidth to review, I'd appreciate it.

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


5 years ago

#4 @kadamwhite
5 years ago

  • Focuses rest-api added

#5 @TimothyBlynJacobs
5 years ago

  • Focuses rest-api removed

Why don't we use the render_block function? It looks like the other two filters also won't be fired which will cause inconsistencies.

#6 @TimothyBlynJacobs
5 years ago

  • Focuses rest-api added

#7 follow-up: @kadamwhite
5 years ago

@TimothyBlynJacobs I assumed that the render_block method was deliberately not being used here for some reason. @miinasikk, @matveb @westonruter and others were involved in the initial Gutenberg PR (#5602) and may be able to provide that context.

#8 @westonruter
5 years ago

I'm not sure about that. It may have been because the render_block filter had not been invented yet.

#9 @TimothyBlynJacobs
5 years ago

+1 to committing this then, we could always switch to using render_block later if need, I don't think this blocks that in any way.

What is this line doing in the patch? $block_type = WP_Block_Type_Registry::get_instance()->get_registered( self::$block_name ); I think it could be removed?

#10 @audrasjb
5 years ago

Hi there,

With Beta 3 approaching, we'll need a decision and a commit action very soon.
For Component maintainers: if you don't feel the current patch is ready to land in WP 5.4 in the next few day, it's could be better to move it to 5.5. We're leaving it in the milestone for now.

This ticket was mentioned in Slack in #core by kadamwhite. View the logs.


5 years ago

#12 in reply to: ↑ 7 @miinasikk
5 years ago

@kadamwhite The discussed filters didn't exist at the time of adding this endpoint, and the render_block method was quite small containing just a few reusable lines and would have required a few more extra lines for reusing it, it probably made more sense to just not use it.

That might not be the case anymore though, as far as I remember, no other particular reason for not using the render_block.

Replying to kadamwhite:

@TimothyBlynJacobs I assumed that the render_block method was deliberately not being used here for some reason. @miinasikk, @matveb @westonruter and others were involved in the initial Gutenberg PR (#5602) and may be able to provide that context.

@kadamwhite
5 years ago

Delegate to render_block within block-renderer endpoint

#13 @kadamwhite
5 years ago

  • Keywords commit added

Thank you for the background, @miinasikk ! This is possibly overambitious but seems to both work locally and pass all existing tests, so I'm going to commit a refreshed patch that uses render_block directly per @TimothyBlynJacobs' suggestion.

#14 @kadamwhite
5 years ago

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

In 47360:

REST API: Apply all relevant block rendering filters when rendering block previews.

Several filters were introduced to the render_block method since the initial implementation of the block-renderer/ endpoints, causing the output of those endpoints to diverge from the rendered content of blocks on the frontend.

Props kadamwhite, TimothyBlynJacobs, miinasikk.
Fixes #49387.

Note: See TracTickets for help on using tickets.