Add render field to navigation block experiment

[SantosGuillamot]

What?

I wanted to test how the new render property could work in the Experiment: Replicate Navigation block using directives.

Why?

In that experiment, we are changing a lot of the HTML, and doing it in a string is sometimes complicated. Using the render property could ease the workflow.

How?

I mainly moved the code of the render_block_core_navigation function outside of it, to store the variables. And moved the sprintf of the navigation block to HTML.

[SantosGuillamot]

I just replicated the sprintf of the responsive navigation menu and didn't check the whole code. I think I won't work on this at leats for a few weeks so I decided to publish the PR as a draft in case it is useful or if we want to restart this in the future.

[luisherranz]

This is not straightforward for developers with a JS background, but you can't declare functions in the render file because that file will be called once per block in the page, and functions in PHP are added to the global scope, so the second time it will fail with a "function redeclaration" problem.

You can use any of these approaches, but you'd most likely want to move those declarations to another file and use a require_once.

[SantosGuillamot]

This is not straightforward for developers with a JS background, but you can't declare functions in the render file because that file will be called once per block in the page, and functions in PHP are added to the global scope, so the second time it will fail with a "function redeclaration" problem.

Oh, you're totally right. It breaks if I add two Navigation blocks. Thanks for the tip! It totally makes sense. 🙂

You can use any of these #42430 (comment), but you'd most likely want to move those declarations to another file and use a require_once.

I wanted to quickly test this by moving the functions to a new helper.php, but I found two problems:

• The PHP file is not in the build folder so I had to use something like:

require_once(dirname(dirname(dirname(__DIR__))) . '/packages/block-library/src/navigation/helper.php');

• It now returns a redeclaration error even if there is only one navigation block because it says the functions are already declared in wp-includes/blocks/navigation. Not sure why this happens if I use require_once and not if I declare the functions directly in the index.php.

I guess this would be solved using function_exists, but not sure if it is the best approach. Any ideas?

Well, and I didn't know how to name the new file 😄 . Is there any convention for this?

[luisherranz]

The PHP file is not in the build folder so I had to use something like require_once(dirname(dirname(dirname(__DIR__)))

That's a terrible DX 😄

I don't know how feasible it would be to use Webpack to inspect the render.php file and copy the require'd files to the build folder. Probably quite error-prone.

It now returns a redeclaration error even if there is only one navigation block because it says the functions are already declared in wp-includes/blocks/navigation

Maybe clean your Gutenberg repo (remove build folder and stuff) and try again?

I guess this would be solved using function_exists, but not sure if it is the best approach. Any ideas?

I don't have any other ideas than these ones.

@noisysocks suggested using namespaces, but I don't know how that would play out. Robert, can you explain that in more detail? Thanks 🙂

@ryanwelcher suggested using the plugin.php file. I think that is the best solution for plugins (functions.php in themes).

For Gutenberg core, I think using function_exists would be fine.

[noisysocks]

@noisysocks suggested using namespaces, but I don't know how that would play out. Robert, can you explain that in more detail? Thanks 🙂

They wouldn't help for this case.

Namespaces would be helpful if you were having name conflicts with a method in a different block, e.g. if both Navigation and Post Comments defined a get_content method.

What you probably want here is to put all of the declarations (functions, classes) into a separate file which is imported using require_once. That way the render PHP can run several times but the PHP containing declarations will run only once.

[luisherranz]

Thanks, Robert 🙂

What you probably want here is to put all of the declarations (functions, classes) into a separate file which is imported using require_once. That way the render PHP can run several times but the PHP containing declarations will run only once.

require_once seems like the best option indeed, but the problem is that the DX is not as good because that file is not copied to the /build directory, as Mario pointed out here. Replicating PHP's file resolution in Webpack to identify and iteratively copy all the PHP files required from the render.php file seems a bit complex to me to solve that problem. So I'd rather suggest using the plugin.php file or the function_exists() approach instead.

[SantosGuillamot]

I've just pushed a commit adding the function_exists() check to all the functions, and it seems to work fine even with two navigation blocks.

[noisysocks]

Site note/thought for @gziolo / @adamziel: It's a shame that in practice the DX of using "render" isn't quite as nice as it seemed in theory.

[luisherranz]

I still think it's a big improvement over the render_callback 🙂

[gziolo]

@noisysocks, using a single template file that is included for every block instance is tricky when you embed functions. However, themes run into exactly the same issues and they developed good practices that address that. Shared utility functions are extracted to files that are included once by the theme like functions.php. In the case of functions that generate or print HTML, the more expected approach would be to move the code to the partial template PHP files that can be included as many times as needed.

[noisysocks]

True I support we could rework @wordpress/block-library to have render.php (called by block.json) and functions.php (called once on init) if we wanted to use this pattern more.

[noisysocks]

Another side note 😅: I think something like this proposed API would work really well for blocks like Navigation where there is a huge amount of (fragile) HTML concatenation.

[luisherranz]

I agree. A component system for PHP similar to that one is something we should research at some point 🙂

[gziolo]

Yes, that might help to have something similar to JSX on PHP side 👍