-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
block.json: Allow passing filename as variations
field
#62092
block.json: Allow passing filename as variations
field
#62092
Conversation
f1c7897
to
4329687
Compare
variations
field
Flaky tests detected in fa4c058. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9568198642
|
Size Change: +3.14 kB (+0.18%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
74e0b93
to
dce24d7
Compare
I've added some testing instructions and noticed that isn't quite working yet. I've updated the PR description to reflect that:
AFAICT from the page source, the problem is that the bootstrapped block type has This is probably due to an issue I've seen before where filters aren't applied properly (?) when preloading/bootstrapping 😕 |
I dug a bit deeper, and it seems like the problem on the server side is due to I've checked the corresponding directory, which is the Template Part block's subfolder of the It made me wonder however how we're handling I wonder if that means that using the Anyway, if I'm correct, then the fix would be to include a build step that copies those PHP files over to the build dir. We're already doing that for blocks' Edit: During that build step, we'll also need to prefix any functions inside of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe that shouldn't surprise me, as we don't have a build step anywhere that would copy that file over from the src/ directory 🤔
Yes, it isn't handled in the Gutenberg repository. The same issue most likely would exist if someone would try to use "render": "file:./render.php"
.
I wonder if that means that using the render field in a Core block to specify the file that's supposed to render it would also break 😬
It would require updating the webpack config which shouldn't be that much work but it wasn't necessary so far.
During that build step, we'll also need to prefix any functions inside of the variations.phps with gutenberg_ (much like we do for the index.phps).
I think that's already happening when copying existing index.php
files for core blocks. So that would get extended to cover the linked files through variations
and render
fields. I don't think it's a blocker for this PR.
At the same time, I think it would be worth adding support for variations file in @wordpress/scripts
mirroring what we have for render
path:
new RenderPathsPlugin(), |
gutenberg/packages/scripts/config/webpack.config.js
Lines 52 to 57 in 82598f9
/** | |
* The plugin recomputes the render paths once on each compilation. It is necessary to avoid repeating processing | |
* when filtering every discovered PHP file in the source folder. This is the most performant way to ensure that | |
* changes in `block.json` files are picked up in watch mode. | |
*/ | |
class RenderPathsPlugin { |
gutenberg/packages/scripts/config/webpack.config.js
Lines 367 to 379 in 82598f9
{ | |
from: '**/*.php', | |
context: getWordPressSrcDirectory(), | |
noErrorOnMissing: true, | |
filter: ( filepath ) => { | |
return ( | |
process.env.WP_COPY_PHP_FILES_TO_DIST || | |
RenderPathsPlugin.renderPaths.includes( | |
realpathSync( filepath ).replace( /\\/g, '/' ) | |
) | |
); | |
}, | |
}, |
It probably would require making the existing custom plugin general purpose tool for discovering linked PHP files.
dce24d7
to
b745e05
Compare
@gziolo Thank you for the pointers! I dug a bit and found that #43917 actually did add support for copying Edit: To clarify, this only applies to third-party blocks, but not to Core blocks (which use a different build system). |
I've filed #63077 to document that |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I've started work on including |
… be filePaths (strings) as well
Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
3753426
to
0ea4266
Compare
schemas/json/block.json
Outdated
"type": "string" | ||
"oneOf": [ | ||
{ | ||
"type": "string" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it is worth adding a description
property here to better document the expected usage of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a good point. I thought it might collide with the description
a couple lines above (i.e. which applies to both oneOf
options), but it seems to be allowed by the JSON schema spec, so I'll add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've started work on including variations files in the build (for third-party blocks): #63098
Ok, that work for me as long it's covered 👍🏻
I don't have more comments on my side, but I see some pending feedback to address from @tjcafferkey before merging.
Thank you very much @gziolo and @tjcafferkey! |
…2092) Allow passing the name of a PHP file that returns a block's variations in the `block.json` file's `variations` field. Co-authored-by: ockham <bernhard-reiter@git.wordpress.org> Co-authored-by: tjcafferkey <tomjcafferkey@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
…2092) Allow passing the name of a PHP file that returns a block's variations in the `block.json` file's `variations` field. Co-authored-by: ockham <bernhard-reiter@git.wordpress.org> Co-authored-by: tjcafferkey <tomjcafferkey@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
Hey @ockham 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
Hey @fabiankaegy, sure thing! Please find my suggested Dev note for this enhancement here. |
What?
Modeled after https://core.trac.wordpress.org/changeset/54132/, which introduced the
render
field forblock.json
, as a way to point to a PHP file instead of providing arender_callback
.The main difference to the
variations
andvariation_callback
fields is that thevariations
field already existed prior to this PR, and it can be used to provide the static list of variations (i.e., an array). This PR makes it so that the field can be alternatively set to a string, which will be interpreted as the filename of the PHP file generating the variations.Due to the changes to the
block.json
schema, it is recommended to view the diff with whitespace changes hidden.Why?
See WordPress/wordpress-develop#6668.
How?
In the GB compat layer: By using the
block_type_metadata_settings
filter.Testing Instructions
Apply the following patch, and create
build/block-library/blocks/template-part/variations.php
with the content below.Patch
build/block-library/blocks/template-part/variations.php
(The latter is required since Gutenberg isn’t currently copying files specified in a
block.json
’srender
orvariations
field to thebuild/
directory. Refer to #63077 for further information.)Then, in the Site Editor, verify that instances of the block still work, and that their variations are detected correctly (e.g. a header template part is recognized as such in the block inspector).
Specifically, open the inserter and enter "Header" into the search input field. Both the Template Part block and its Header variation should be shown.