3 gutenberg_-functions called in wp-trunk (blocks/navigation.php)

[kebbet]

Description

There are three instances in the navigation block where gutenberg_-functions are called. They should be replaced with their correct function name, and be fixed upstream. Introduced in https://core.trac.wordpress.org/changeset/52069.

File: /src/wp-includes/blocks/navigation.php
L176: gutenberg_get_menu_items_at_location
L181: gutenberg_sort_menu_items_by_parent_id
L182: gutenberg_parse_blocks_from_menu_items

A ticket for the same issue has been created in the WordPress trac system. https://core.trac.wordpress.org/ticket/54532

Step-by-step reproduction instructions

The functions seems to exist in trunk but under other names, such as _wp_get_*

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

[noisysocks]

These functions were recently removed from Core (WordPress/wordpress-develop@a9e66f4) but remain in the plugin (https://github.com/WordPress/gutenberg/blob/trunk/lib/navigation.php).

Are they still being used by the Navigation block? If not, we can simply remove this code.

Otherwise, I'm not sure what to do. Ordinarily we would gate functionality that needs to exist in the plugin but not in Core behind the GUTENBERG_PHASE variable. But that doesn't work in PHP. Perhaps we have to implement something analogous to GUTENBERG_PHASE for block PHP.

What do you think @talldan?

[tellthemachines]

#36604 removes those functions from Gutenberg. It's approved, but has been sitting there for a few days and has a bunch of conflicts. From the convo there was uncertainty about whether this should go into 5.9, but given it removes functionality that shouldn't be in core, backporting it would fix the issue here.

[talldan]

These were already removed for the navigation block in the plugin in #36336. lib/navigation.php isn't the block's PHP. That's in the block-library package.

[tellthemachines]

@talldan those functions are still in the latest trunk, e.g. here

[talldan]

Ok, @noisysocks has corrected me, and it's not so much that the functions exist, but that they are called.

[talldan]

@talldan those functions are still in the latest trunk, e.g. here

Yeah, but they shouldn't be deleted, they're in use in the plugin. Functions in that file will be renamed when copied to trunk.

The issue is that the block's php file is copied automatically and so the call site of those functions is wrong.

[talldan]

So for some more background, these functions are for the __unstableLocations feature in the nav block, it's used to render a classic menu on the front end of a user's site. It's a plugin only feature that still needs some work.

They were also used by navigation area block, but the navigation area feature has been or will be removed.

So I think the quandry is how do we handle unstable code given the block php is automatically copied across to core.

Now that they're only used by the navigation block I think the following could be possible.

• Move the functions back into the block's php file and rename them with a _block_core_navigation prefix
• Define a constant like IS_GUTENBERG_PLUGIN in the gutenberg plugin
• Wrap the functions in an if statement that checks defined( IS_GUTENBERG_PLUGIN )
• Also gate where the functions are called using the same check.

[noisysocks]

Move the functions back into the block's php file

This bit isn't strictly necessary. Gating the part of navigation/index.php which calls these functions makes sense though.

[talldan]

This bit isn't strictly necessary. Gating the part of navigation/index.php which calls these functions makes sense though.

True, I'd just prefer to have the function references be correct so that they can be traced to the callsite.