Block Library: Update FSE blocks to use block context #21696
Conversation
aduth
added
[Package] Block library
aduth
requested review from
ajitbohra,
jorgefilipecosta,
Soean,
talldan and
TimothyBJacobs
as code owners
|
Size Change: +8 B (0%) Total Size: 822 kB
ℹ️ View Unchanged
|
|
Nice work! We should also update the client-side implementations in this PR. |
|
I'm currently working to rebase this after having merged #21925, and I'm encountering an issue I expect likely had existed previously but I failed to notice. Basically, the site errors with a memory exhausted error when trying to load the Site Editor page. I have a feeling it's related to the fact that we're not longer "taking advantage of" this fix: Lines 167 to 171 in 2beb598 I guess we need something more permanent here... I still need to dig in more. Curious if you have any thoughts @epiqueras. My first intuition is:
|
|
The template should not be setting its ID and post type into context. |
As I understand it, this is the current behavior, but it's also not unsetting it either, so when it's attempting to render a Post Content block within the template, the block inherits the ID of the template, which in turn renders the template, which in turn renders the Post Content block within the template, inherits the same template ID, ... (ad infinitum). |
|
That's why the top-level post ID and post type should not be set when rendering templates or template parts. I.e., in |
aduth
force-pushed
the
update/21662-deprecate-get-post-from-context
branch
from
16c8b30
to
8193c87
Compare
Yeah, it makes sense. It took me some time to think it through, but since the template is meant to describe a block arrangement for any post which matches that template, it makes sense that we should want to avoid assigning post values into block context until the point at which that template rendering occurs. I was reluctant to put a hard-coded condition here in I think where I landed with a862753 and 9a837f3 is a happy medium: The post context is there by default, but the default context can be filtered. The code (file) relevant for template rendering hooks this filter to unset the post context when a template post is being rendered. |
|
In the course of refactoring these blocks, it's been interesting to note there have been several occurrences where we previously unknowingly depended on the Examples:
|
Given the additional challenges which have arisen here already out of scope (supporting filterable context, unsetting post context for templates, restoring E2E tests, fixing blocks depending on Personally, I'd rather do a quick follow-on after this has landed. |
|
Sounds good. |
Presumably the previous implementation relied on the fact that calling gutenberg_get_post_from_context would reset post global, referenced internally in get_the_content implicitly by call to get_post
With testing of availability of default post contxt
aduth
force-pushed
the
update/21662-deprecate-get-post-from-context
branch
from
74cc973
to
63f66c3
Compare
Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>
|
LGTM |
Closes #21662
Closes #21665
This pull request seeks to update existing full-site editing blocks to use the block context feature implemented in #21467 in place of the existing
gutenberg_get_post_from_contextfor accessing the "current" post. In so doing, thegutenberg_get_post_from_contextfunction has been deprecated.It does not seek to make any other improvements to these blocks. Existing functionality is retained.
Testing Instructions:
Verify all affected blocks behave as expected:
core/post-contentcore/post-authorcore/post-commentscore/post-comments-countcore/post-comments-formcore/post-datecore/post-excerptcore/post-featured-imagecore/post-tagscore/site-titlecore/template-partThis requires to create a template using the Full Site Editing feature, enabled under Gutenberg > Experiments. The easiest way to do this is to create a new template
single, add each of these as a block somewhere in the template, then navigate to the preview of an existing post on the site.Ideally there are automated tests for these blocks. I will create a separate tracking task for this.