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

Blocks: Add new render field to block.json #42430

Merged
merged 2 commits into from
Aug 30, 2022
Merged

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jul 14, 2022

What?

A technical prototype for Editor: Consider adding render property for block types in WordPress Trac. I will add tests and documentation only when people find this proposal useful.

Related to the proposal #41289 from @ryanwelcher to add --is-dynamic flag to @wordpress/create-block.

Why?

See comment from @spacedmonkey #13693 (comment).

Current the render callback is a function call. How would it work as a file? Why not make it renderTemplate but still allow for renderCallback. Function calls, should be a method in a class or be namespaced, which wouldn't really work in another file.

ACF blocks, has a rendercallback / render template in it's block definitions.

There is also another thread started by @spacedmonkey in #13693 (comment) which expands on this topic.

How?

New render field in block.json file that accepts a string value.

The path to the file should be prefixed with file: and be relative to the block.json file in the build (target) folder. It's exactly the same pattern as used with scripts and styles in block.json.

PHP file that renders the template for the block can use 3 variables that are the same as render_callback takes as params:

  • $attributes (array) - block attributes.
  • $content (string) - block default content.
  • $block (WP_Block) - block instance.

Important note: I refactored the Archive block to use the new render field in block.json only for testing purposes. We rather won't do it as using function declarations is problematic with this approach because template files sometimes need to load several times for a given block type. When that happens, then we encounter a function redeclaration error. Removed from PR.

Testing Instructions

Ensure that the temporarily refactored Archive blocks still work as before.

To test with custom blocks you need to add render field in the block.json file.

Example:

src/block.json

{
    "name": "test/my-block",
    "render": "file:./render.php"
}

src/render.php

<div <?php echo get_block_wrapper_attributes(); ?>>
    <?php echo $attributes['text']; ?>
</div>

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Type] New API New API to be used by plugin developers or package users. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible labels Jul 14, 2022
@gziolo gziolo force-pushed the new/block-render-template branch from fd9dcef to f1f1b37 Compare July 14, 2022 13:37
@gziolo gziolo self-assigned this Jul 15, 2022
@gziolo gziolo marked this pull request as ready for review July 18, 2022 10:23
@gziolo gziolo requested review from a team, noisysocks and talldan and removed request for a team July 18, 2022 10:46
@luisherranz
Copy link
Member

This is very interesting 🙂

I'm trying to figure out what would be the DX of adding functions without facing the redeclaration error:

  • Move the logic to a different file and use require.
  • Move the function to another file and use require_once.
  • Use function_exists.
    if ( ! function_exists( 'my_dynamic_block_logic' ) ) {
      function my_dynamic_block_logic( $params ) use ( $attributes ) {
        // ... logic here
      }
    }
    
    my_dynamic_block_logic( 123 );
  • Use anonymous functions
    $my_dynamic_block_logic = function ( $params ) use ( $attributes ) {
      // ... logic here
    }
    
    $my_dynamic_block_logic( 123 );

Is there any other option?

@ryanwelcher
Copy link
Contributor

ryanwelcher commented Jul 18, 2022

IMO, there really shouldn't be any function declarations in the template file at all. This file can be conceptually compared to files that would be used with get_template_part() in that they usually only contain function calls and markup.

This is definitely a case of making sure there is proper documentation and best practices around this feature but I think that any functionality this block requires is better defined in the plugin.php or functions.php file - depending on whether the block is registered from a theme or plugin.

@luisherranz
Copy link
Member

I think that any functionality this block requires is better defined in the plugin.php or functions.php file

Yeah, I guess that makes sense from a WordPress point of view.

Sorry, I was thinking from the perspective of devs without much WP/PHP background that want to componentize their markup logic, so they just start adding functions to those entry points. And probably thinking of a future where just adding a bunch of block.json files is enough (no plugin.php nor functions.php required) 😅

@noisysocks
Copy link
Member

Is there any other option?

Could use a namespace? Agree with @ryanwelcher though that not putting declarations in these files feels more WordPress-ey.

@gziolo
Copy link
Member Author

gziolo commented Jul 19, 2022

The example included in the Blocks ACF plugin doesn't contain any function and class declarations:
https://www.advancedcustomfields.com/resources/blocks/#introduction

IMO, there really shouldn't be any function declarations in the template file at all. This file can be conceptually compared to files that would be used with get_template_part() in that they usually only contain function calls and markup.

So that explanation makes perfect sense here. It's also similar to what you can observe in templates for themes, for example:

https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-content/themes/twentyeleven/single.php

@gziolo gziolo mentioned this pull request Jul 20, 2022
67 tasks
@mtias
Copy link
Member

mtias commented Jul 26, 2022

The name is a bit weird to me. What would be the relationship between a renderTemplate field and a view or frontend field as discussed in WordPress/block-interactivity-experiments#37 ?

@ryanwelcher
Copy link
Contributor

ryanwelcher commented Jul 26, 2022

The name is a bit weird to me. What would be the relationship between a renderTemplate field and a view or frontend field as discussed in WordPress/block-hydration-experiments#37 ?

@mtias IMO as a PHP-first developer, renderTemplate aligns with the idea of a file that we would require inside the render_callback function - here's an example from one of my last streams.

It is also closer in naming to get_template_part() which again, resonates with developers used to including templates into PHP files in WordPress.

@mtias
Copy link
Member

mtias commented Jul 27, 2022

The semantics of renderTemplate doesn't make much sense next to edit and save because those also represent templates

@gziolo
Copy link
Member Author

gziolo commented Jul 27, 2022

The semantics of renderTemplate doesn't make much sense next to edit and save because those also represent templates

Interesting observation. I didn't consider that at all. I guess the render_callback method that existed from the beginning is misleading here. When writing PHP code, that name isn't that problematic because edit and save exist as concepts only on the client side. We already have view reserved in the block.json file for JavaScript that should be loaded only on the frontend. Maybe viewTemplate could be a good alternative? As of today, it would support PHP templates only. If one day, we will figure out the format that works cross-platform as summarized in Universal blocks and block hydration, then we just wire it with viewTemplate. What do you think?

@mtias
Copy link
Member

mtias commented Jul 27, 2022

render_callback happens directly in the PHP context so it makes more sense. If we are modelling something for block.json we need to take all environments into account for API clarity. In the context of edit, save, and view, perhaps render would be an ok designation, or server otherwise. I think "view" should eventually coalesce for server / client, so not opposed to exploring something where view could be a "file:./template.php" or a JS function. In some of the single file explorations from @luisherranz we have that dichotomy present.

@gziolo
Copy link
Member Author

gziolo commented Aug 2, 2022

. I think "view" should eventually coalesce for server / client, so not opposed to exploring something where view could be a "file:./template.php" or a JS function. In some of the single file explorations from @luisherranz we have that dichotomy present.

That's an interesting take. We have only viewScript at the moment. We have already received some feedback that viewStyle would be helpful as well. Here, we would have something closer to viewRender or viewServer. We could definitely consolidate all that under one view field in the block.json and handle everything behind the scenes:

{
    "view": [ "file:./template.php", "file:./view.js", "file:./view.css" ]
}

It would be less straightforward when passing script or style handles, but we could always run some detection whether this handle is for a style, a script, or both.

I would love to hear what @luisherranz thinks about all these ideas in the context of the block hydration experiments.

@luisherranz
Copy link
Member

I would love to hear what @luisherranz thinks about all these ideas in the context of the block hydration experiments.

Categorizing the PHP rendering as view feels a bit confusing to me because Gutenberg uses that term to designate what is sent to the browser. It's true that in the block hydration experiments, we're trying to get rid of save. But it's not clear yet to me if/how that would happen, especially for dynamic blocks.

Today, technically speaking, the PHP rendering is the dynamic equivalent of the save.js, but the save action (saving to the database) doesn't apply to PHP. On the other hand, the render action describes what is happening on the PHP side. So I don't have a strong opinion at the moment, and I'm not sure yet if the PHP rendering will fit better in the save or the view category, or if it will be better to rename save to render instead in the future.

Anyway, if I had to vote, I'd use the render name for now to keep the status quo, knowing that we can rename it in a future block.json version (along with other inconsistencies), but I'll also be happy with any other decision 🙂

@gziolo
Copy link
Member Author

gziolo commented Aug 8, 2022

Anyway, if I had to vote, I'd use the render name for now to keep the status quo, knowing that we can rename it in a future block.json version (along with other inconsistencies), but I'll also be happy with any other decision 🙂

It's only processed on the server, so we shouldn't be worried about renaming that later and even keeping it backward-compatible. I'm fine with using render in block.json. It's also one of the options that @mtias mentioned in his comment.

Today, technically speaking, the PHP rendering is the dynamic equivalent of the save.js, but the save action (saving to the database) doesn't apply to PHP.

It's more nuanced. We have more places these days where render_callback uses the content produced by save and further processes it before sending it to the user.

@gziolo
Copy link
Member Author

gziolo commented Aug 8, 2022

I updated this branch upon the feedback received. I included documentation changes necessary for the new render field in the block.json file that'd be available starting from WordPress 6.1.0 in core.

Before merging this PR, I will have to remove changes from the Archive block, as noted in the description. Everything else should be ready for the final feedback.

@gziolo gziolo added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Aug 8, 2022
@gziolo gziolo changed the title Blocks: Add new renderTemplate field to block.json Blocks: Add new render field to block.json Aug 10, 2022
@gziolo
Copy link
Member Author

gziolo commented Aug 25, 2022

I removed the refactoring applied to the Archive block that was presenting the idea. I think this PR received enough feedback so let's make the final decision whether it's good enough to get included in WordPress 6.1.

Copy link
Member

@fabiankaegy fabiankaegy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this API is very valuable. In the scaffolding we use for custom block development we already have a similar abstraction which we could drop in favor of the core approach if this were to land.

Also, thank you @gziolo for writing such great documentation for this feature! :) It is really appreciated <3

Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
Copy link
Contributor

@adamziel adamziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks and works great, thank you @gziolo!

Here's a caveat for a follow-up PR: The Gutenberg plugin build process does not bundle the render.php files in the build directory, which means the render scripts cannot be used with core blocks yet.

@gziolo
Copy link
Member Author

gziolo commented Aug 30, 2022

Here's a caveat for a follow-up PR: The Gutenberg plugin build process does not bundle the render.php files in the build directory, which means the render scripts cannot be used with core blocks yet.

We don't necessarily need to immediately refactor any of the existing core blocks to use render. However, we definitely can update the webpack config to read this new field in block.json to discover the exact file path used. This way, we would no longer need to harcode the name of the PHP file processed during the build and remember to update the list of dynamic blocks in:

'block_names' => array(

@gziolo gziolo merged commit 132b86c into trunk Aug 30, 2022
@gziolo gziolo deleted the new/block-render-template branch August 30, 2022 15:57
@github-actions github-actions bot added this to the Gutenberg 14.1 milestone Aug 30, 2022
@gziolo
Copy link
Member Author

gziolo commented Aug 30, 2022

I landed the current version, but I'd like to double-check what do you think about making it possible to pass also the callback name in the render field like we do it today with render_callback on the server with register_block_type?

@Rahe
Copy link
Contributor

Rahe commented Aug 30, 2022

Hello,

Allowing multiple values for this render attribute could be possible ? and use an get_template_part like function ?

This will allow to redefine Dynamic blocs, even core ones, within themes.
Like ["theme:./blocs/my-template.php","./render.php"]
In this exemple the theme file on child/parent will be searched, then fallback on plugin file.

This will allow to have a template hierarchy like rendering process for Dynamic blocs.

Nicolas,

@gziolo
Copy link
Member Author

gziolo commented Sep 9, 2022

Allowing multiple values for this render attribute could be possible ? and use an get_template_part like function ?

Not at the moment. Similar to how render_callback with register_block_type works today, you can pass only a single file path.

This will allow to redefine Dynamic blocs, even core ones, within themes. Like ["theme:./blocs/my-template.php","./render.php"] In this exemple the theme file on child/parent will be searched, then fallback on plugin file.

This will allow to have a template hierarchy like rendering process for Dynamic blocs.

Interesting idea. You can get close to that with WP hook. See block_type_metadata. This way, you can provide your templates within a plugin or theme by overriding the value provided in render. Although the challenge would be that the path is relative to block.json file.

pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Sep 12, 2022
New `render` field in `block.json` file that accepts a string value. It allows to pass a path to the PHP file that is going to be used to render the block on the server.  Related PR in Gutenberg: WordPress/gutenberg#42430.

Props spacedmonkey, luisherranz, welcher, noisysocks, matveb, fabiankaegy, aristath, zieladam.
Fixes #53148.



git-svn-id: https://develop.svn.wordpress.org/trunk@54132 602fd350-edb4-49c9-b593-d223f7449a82
@gziolo
Copy link
Member Author

gziolo commented Sep 12, 2022

I backported this new API to WordPress core with https://core.trac.wordpress.org/changeset/54132.

markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Sep 12, 2022
New `render` field in `block.json` file that accepts a string value. It allows to pass a path to the PHP file that is going to be used to render the block on the server.  Related PR in Gutenberg: WordPress/gutenberg#42430.

Props spacedmonkey, luisherranz, welcher, noisysocks, matveb, fabiankaegy, aristath, zieladam.
Fixes #53148.


Built from https://develop.svn.wordpress.org/trunk@54132


git-svn-id: http://core.svn.wordpress.org/trunk@53691 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this pull request Sep 12, 2022
New `render` field in `block.json` file that accepts a string value. It allows to pass a path to the PHP file that is going to be used to render the block on the server.  Related PR in Gutenberg: WordPress/gutenberg#42430.

Props spacedmonkey, luisherranz, welcher, noisysocks, matveb, fabiankaegy, aristath, zieladam.
Fixes #53148.


Built from https://develop.svn.wordpress.org/trunk@54132


git-svn-id: https://core.svn.wordpress.org/trunk@53691 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@bph bph mentioned this pull request Sep 14, 2022
89 tasks
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this pull request Sep 18, 2022
New `render` field in `block.json` file that accepts a string value. It allows to pass a path to the PHP file that is going to be used to render the block on the server.  Related PR in Gutenberg: WordPress/gutenberg#42430.

Props spacedmonkey, luisherranz, welcher, noisysocks, matveb, fabiankaegy, aristath, zieladam.
Fixes #53148.


Built from https://develop.svn.wordpress.org/trunk@54132
ootwch pushed a commit to ootwch/wordpress-develop that referenced this pull request Nov 4, 2022
New `render` field in `block.json` file that accepts a string value. It allows to pass a path to the PHP file that is going to be used to render the block on the server.  Related PR in Gutenberg: WordPress/gutenberg#42430.

Props spacedmonkey, luisherranz, welcher, noisysocks, matveb, fabiankaegy, aristath, zieladam.
Fixes #53148.



git-svn-id: https://develop.svn.wordpress.org/trunk@54132 602fd350-edb4-49c9-b593-d223f7449a82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. Needs Dev Note Requires a developer note for a major WordPress release cycle [Type] New API New API to be used by plugin developers or package users. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants