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

Functions prefixed with `gutenberg_´ in WP6.3 beta 2. #52103

Closed
kebbet opened this issue Jun 29, 2023 · 10 comments
Closed

Functions prefixed with `gutenberg_´ in WP6.3 beta 2. #52103

kebbet opened this issue Jun 29, 2023 · 10 comments
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@kebbet
Copy link
Contributor

kebbet commented Jun 29, 2023

Description

With WordPress 6.3 beta 2, two function were included that are prefixed with gutenberg_. They are: gutenberg_block_core_navigation_update_interactive_view_script and gutenberg_block_core_file_update_interactive_view_script. Their name shoul be updated to not use that prefix.

Step-by-step reproduction instructions

See WordPress beta 2 source code.

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 6.3 beta 2
  • No plugins installed.

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

@ndiego
Copy link
Member

ndiego commented Jul 3, 2023

cc @tellthemachines @ramonjd

@ndiego ndiego added the [Type] Bug An existing feature does not function as intended label Jul 3, 2023
@ndiego ndiego moved this from ❓ Triage to 📥 Todo in WordPress 6.3.x Editor Tasks Jul 3, 2023
@ramonjd
Copy link
Member

ramonjd commented Jul 3, 2023

It appears that both these functions are Gutenberg-only, experimental functionality (for the interactivity API) that are wrapped in the following condition:

if ( defined( 'IS_GUTENBERG_PLUGIN' ) && IS_GUTENBERG_PLUGIN ) {

It means they'll only run when the plugin is installed and active.

Should we removex everything wrapped in the IS_GUTENBERG_PLUGIN constant check in Core? It would have the same effect either way (none).

If I were strapped to a rocket and given 2 seconds to decide the fate of this code, I'd probably lean towards removing it. Only because a JS equivalent exists, which is used at build time process.env.IS_GUTENBERG_PLUGIN, however, the code is, I think, removed during compilation.

If folks think this is the right way to go I can prep a PR for the wp/6.3 branch.

@tellthemachines
Copy link
Contributor

Anything wrapped in IS_GUTENBERG_PLUGIN is safe to leave in core; there are multiple occurrences already and it's handled in the build on the core side.

It might be good to update the gutenberg_ function prefixes to wp_ all the same, as that's standard practice in block PHP files on the gutenberg side, similar to what was done in #51989.

@ramonjd
Copy link
Member

ramonjd commented Jul 4, 2023

It might be good to update the gutenberg_ function prefixes to wp_ all the same, as that's standard practice in block PHP files on the gutenberg side

I think these functions have no equivalent in Core since they're still experimental plugin features (?)

Never mind. Brain meltdown, sorry. I agree we could rename the functions 😄

It's not quite the same as in #51989, however, which only renamed to an existing core function. Here we are introducing two new, currently experimental, functions.

@kebbet
Copy link
Contributor Author

kebbet commented Jul 4, 2023

If it's Gutenberg specific code, why put in WordPress core?

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jul 4, 2023
@ramonjd ramonjd moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.3.x Editor Tasks Jul 4, 2023
@ntsekouras
Copy link
Contributor

If it's Gutenberg specific code, why put in WordPress core?

The blocks code is built automatically in WP core folder structure. There are some features/code that needs to be part of the php blocks' code, but we don't want it to be part of WP core. My understanding is that is the approach to handle such cases as @tellthemachines also notes.

@ramonjd
Copy link
Member

ramonjd commented Jul 5, 2023

These functions have been removed in Gutenberg trunk in 9b8d5c1#diff-adf84f1d0ae56974f4c9f23d5d07d3e1fe9f3bda38d4bd2d8ba13ba41d9f0e70 already

They still exist in Core trunk, e.g., here

@annezazu
Copy link
Contributor

I'm confused as to the state of this issue 😅. Is there more needed for 6.3? It seems like not based on the various discussions across this issue and #51077 but I wanted to be sure!

@ramonjd
Copy link
Member

ramonjd commented Jul 24, 2023

I think the decision was to leave them to preserve consistency with block PHP files.

#51077 (comment)

They don't run in Core, only in the plugin.

Block PHP files are not manually backported, but are synced via the package updates. Since they are pulled in "as-is", there is therefore no opportunity to "massage" them into the Core code base.

Any name change would need to occur in the plugin.

So gutenberg_block_core_navigation_update_interactive_view_script => wp_block_core_navigation_update_interactive_view_script

At least that's the way I've understood the feedback. I get the impression it's okay to close this off since folks seem fine with that.

@ramonjd
Copy link
Member

ramonjd commented Jul 25, 2023

Closing for now because:

  • There are no apparent side effects for Core, despite the apparent inconsistency with the way we handle such function names in Core backports
  • Until the protocol around block packages and Gutenberg-only code becomes clearer

@ramonjd ramonjd closed this as completed Jul 25, 2023
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.3.x Editor Tasks Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

No branches or pull requests

6 participants