Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

register_block_core_navigation_link should not be do as init hook #49678

Open
obache opened this issue Apr 10, 2023 · 6 comments
Open

register_block_core_navigation_link should not be do as init hook #49678

obache opened this issue Apr 10, 2023 · 6 comments
Labels
[Block] Navigation Link Affects the Navigation Link Block [Type] Enhancement A suggestion for improvement.

Comments

@obache
Copy link

obache commented Apr 10, 2023

Description

register_block_core_navigation_link() of navigation-link block will use registered post_type and taxonomy, then register variations of navigation-link block. It will be done as init hook with default priority now.

But from functions references

register_post_type(): Post type registrations should not be hooked before the ‘init’ action.
register_taxnomy(): Do not use before the ‘init’ hook.

So custom post types and taxonomies may not be registered yet at register_block_core_navigation_link() doing, and such post type links and taxonomy archive links are not appeared as blocks for navigation menu.

In "Registering Custom Post Types" of "Plugin Handbook", custom post type is registered as init hook with default priority 10, so it should be usual timing to register custom post type for plugins , but archive link for such registered post type will not be appeared as blocks. If post types is registered as init hook with priority 9, such post types will be appeared.

register_block_core_navigation_link() should be done after init hook, or lower priority.

Step-by-step reproduction instructions

  1. register a custom post type in init hook with default priority
  2. add navigation menu in Post Edit admin screen
  3. in navigation block settings "Menu", try to add block and "Browse all blocks" in left side navi "block inserter"
  4. registered custom post type is not in "THEMES" blocks.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@kathrynwp kathrynwp added [Block] Navigation Link Affects the Navigation Link Block [Type] Enhancement A suggestion for improvement. Needs Testing Needs further testing to be confirmed. labels Apr 10, 2023
@ndiego
Copy link
Member

ndiego commented Apr 11, 2023

I have been able to replicate this issue. For reference, this feature was introduced in #29095.

@ndiego ndiego removed the Needs Testing Needs further testing to be confirmed. label Apr 11, 2023
@gziolo gziolo added the [Priority] High Used to indicate top priority items that need quick attention label May 24, 2023
@gaambo
Copy link
Contributor

gaambo commented Nov 13, 2023

FYI: This has been fixed in #54801

@obache
Copy link
Author

obache commented Nov 14, 2023

How about unregister_post_type() and unregister_taxonomy() ?

@gaambo
Copy link
Contributor

gaambo commented Nov 14, 2023

How about unregister_post_type() and unregister_taxonomy() ?

@obache That's a great point that I've missed. I've added a new PR which fixes this as well: #56100

@annezazu
Copy link
Contributor

annezazu commented Dec 19, 2023

As part of this larger sweep of high priority issues, I'm removing the label here as the main part of the issue has been resolved with an additional PR in progress and already approved to iterate. Re-running the jobs to see if we can get that merged sooner rather than later. Let me know if you need more help getting this through @gaambo too :)

@annezazu annezazu removed the [Priority] High Used to indicate top priority items that need quick attention label Dec 19, 2023
@gaambo
Copy link
Contributor

gaambo commented Jan 12, 2024

#56100 is now merged so unregistering post types / taxonomies also works correctly now 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants