Make WordPress Core

Opened 2 years ago

Closed 8 months ago

Last modified 4 months ago

#55655 closed defect (bug) (invalid)

6.0: Throw a _doing_it_wrong in register_block_pattern if not registered on `init` hook.

Reported by: johnstonphilip's profile johnstonphilip Owned by:
Milestone: Priority: normal
Severity: normal Version: 6.0
Component: Editor Keywords: has-patch has-unit-tests dev-feedback changes-requested close
Focuses: administration, rest-api, coding-standards Cc:

Description

In WP 6.0, block patterns are now being loaded after initial page load via the REST API.

This means that any block patterns not registered on the init hook will no longer show up.

Therefore, the register_block_pattern function should only be used on the init hook, and if not, it should throw a _doing_it_wrong so the code author knows.

For more see:
https://github.com/WordPress/gutenberg/issues/40736

Change History (20)

#1 @costdev
2 years ago

  • Component changed from General to Editor
  • Milestone changed from Awaiting Review to 6.0
  • Summary changed from Throw a _doing_it_wrong in register_block_pattern if not registered on `init` hook. to 6.0: Throw a _doing_it_wrong in register_block_pattern if not registered on `init` hook.

Thanks @johnstonphilip!

Milestoning for 6.0 for visibility and because the need for this was introduced during the 6.0 cycle (i.e. I'm considering it a regression without the _doing_it_wrong()).

This ticket was mentioned in PR #2662 on WordPress/wordpress-develop by costdev.


2 years ago
#2

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

hellofromtonya commented on PR #2662:


2 years ago
#3

See my comments here https://github.com/WordPress/gutenberg/issues/40736#issuecomment-1115494306 about forcing registration to only be when hooked to init. I'm wondering what the impact to extenders will be.

peterwilsoncc commented on PR #2662:


2 years ago
#4

See my comments here WordPress/gutenberg#40736 (comment) about forcing registration to only be when hooked to init. I'm wondering what the impact to extenders will be.

I think it will be problematic.

Both post types and taxonomy require developers wait until the _init hook or later_ to register a custom object. As the init hook is relatively early, later registration can come in handy when the developer wishes to know something about the request when registering the custom object.

That may also be the case here where waiting for the query to be parsed may be helpful.

#5 @peterwilsoncc
2 years ago

I put the Gutenberg issue on the WP 6.0 project board for the editor folks to action.

I'll keep this ticket open for now as it looks like the code might need to be manually backported to WP.

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


2 years ago

#7 @hellofromTonya
2 years ago

  • Milestone changed from 6.0 to 6.1

6.0 RC1 is minutes away. Given there are BC concerns about making init a hard requirement, moving this to 6.1. It could be considered for a minor too.

#8 @johnstonphilip
2 years ago

@hellofromTonya I don't believe there are any BC concerns because this is already broken on 6.0 for anyone using a hook other than init. The _doing_it_wrong will at least offer an explanation as to why it is broken.

Last edited 2 years ago by johnstonphilip (previous) (diff)

#9 @hellofromTonya
2 years ago

To help contributors and followers, the main conversation for this issue is in this Gutenberg issue https://github.com/WordPress/gutenberg/issues/40736.

For the BC concerns, please refer to @peterwilsoncc's comment in that issue:

WordPress has a core philosophy to maintain backward compatibility, so neither a doing it wrong call or saying it's not possible is the answer. Something needs to be fixed.

bdd024e removed examples of Gutenberg registering block patterns on hooks other than init. Extenders learnt to use current_screen from Core & Gutenberg.

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


20 months ago

#11 @audrasjb
20 months ago

Just updated the PR to change @since mentions.

As per today's bug scrub, it appears the related upstream PR was merged.

@peterwilsoncc @costdev do you think this PR could ship with 6.1?

#12 @desrosj
20 months ago

  • Milestone changed from 6.1 to 6.2

Unfortunately, this one won't make it. RC1 is due out within the hour.

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


16 months ago

@hellofromTonya commented on PR #2662:


15 months ago
#14

Notice at the end of WP_Block_Patterns_Registry::register() there's code to detect and store a pattern not registered on init. The code in this PR would bail out before that code and the subsequent code that works with the registered_patterns_outside_init property. In other words, a _doing_it_wrong() with an early bail-out negates the tracking and use case of the registered_patterns_outside_init property and its usage.

#15 @hellofromTonya
15 months ago

  • Keywords dev-feedback changes-requested added

#16 @hellofromTonya
15 months ago

  • Keywords close added

Correct me if I'm wrong, but reading the resolution of https://github.com/WordPress/gutenberg/issues/40736, the fix implemented was:

A combination of https://github.com/WordPress/wordpress-develop/pull/2672 (Core) and https://github.com/WordPress/gutenberg/pull/40818 (Gutenberg) should fix this.

[53404] / #55567 added tracking and handling of patterns registered outside of init action via the registered_patterns_outside_init property. Adding a _doing_it_wrong() would disable that change.

Thinking this ticket then is close as wontfix.

#17 @hellofromTonya
15 months ago

  • Milestone changed from 6.2 to Awaiting Review

Moving this ticket back to Awaiting Review as it needs further discussion and might be a close candidate.

#18 @costdev
15 months ago

I agree with closing the ticket, though maybe invalid and a note that backports in #55567 already solved this issue would be more accurate than wontfix?

#19 @johnstonphilip
8 months ago

  • Resolution set to invalid
  • Status changed from new to closed

#20 @swissspidy
4 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.