Block API: Block Context: Filter content, prepare attributes at render, pass block to render

[aduth]

Related: #21921
Closes #21797

Note: This pull request is intended to be exploratory, related to discussion In #21797 in how best to make available block context and block values in general. Edit: Stability achieved after continued iteration and feedback cycles.

This pull request seeks to explore a few ideas related to block rendering and the provided block value, which is particularly impactful for ongoing work around block context.

It...

• Runs render_block filter in WP_Block::render, intended to address the bug separately addressed in Block API: Block Context: Remove block filter #21921

  • Block API: Add Block Context support #21467 inadvertently regressed the behavior of render_block on inner blocks. The filter is only being run on the top-level blocks of a post content.

  • Observation: Would we expect filters to be run in these otherwise-generic classes? How else could we run a "callback" on the recursive rendered result.
  • Observation: This requires the instance to track its original parsed value, required for backward compatibility since the filter receives it. This may not be necessary if the previous point can be addressed with an alternative implementation.
• Prepares attributes only at time of rendering for a dynamic block.
  • Related to the "overhead" worry described in Block API: Block context render_callback not fully backward-compatible as array #21797 (comment)
  • Observation: You can see even by the tests that this introduces some uncertainty around using values from the $block value, since $block->attributes will be the attributes as parsed, not including any defaults. I haven't yet experimented with it, but the __get magic method usage described in Block API: Block context render_callback not fully backward-compatible as array #21797 (comment) could be a reasonable compromise here.
  • Observation: As named, it makes some sense that the attributes defaulting behavior contained in prepare_attributes_for_render is only called when a block is rendered. However, the proposed benefits of the changes in Block API: Add Block Context support #21467 were partly to soft-deprecate this, and intentionally change to a behavior which was more "predictable" as far as attributes always being defaulted once the class instance is constructed.
  • Observation: Inner blocks mapping, the other constructed logic noted in Block API: Block context render_callback not fully backward-compatible as array #21797 (comment), was not included as part of these changes. It would probably be necessary to keep this in the constructor, at least given how context is currently prepared and passed to descendants using constructor arguments.
• Passes the $block class instance as a third argument of render_callback
  • Observation: It works reasonably well, aside from the above-noted caveat about attributes defaulting if prepare_attributes_for_render is deferred to the point that the block is rendered.

Testing Instructions:

Repeat testing instructions from #21921.

Ensure unit tests pass:

npm run test-unit-php

[github-actions]

Size Change: +4.91 kB (0%)

Total Size: 821 kB

Filename	Size	Change
build/api-fetch/index.js	4.08 kB	-2 B (0%)
build/autop/index.js	2.82 kB	+2 B (0%)
build/block-directory/index.js	6.6 kB	+374 B (5%)	🔍
build/block-editor/index.js	101 kB	-4.77 kB (4%)
build/block-editor/style-rtl.css	10.2 kB	+18 B (0%)
build/block-editor/style.css	10.2 kB	+18 B (0%)
build/block-library/editor-rtl.css	7.08 kB	+55 B (0%)
build/block-library/editor.css	7.08 kB	+58 B (0%)
build/block-library/index.js	115 kB	+1.24 kB (1%)
build/block-library/style-rtl.css	7.24 kB	+100 B (1%)
build/block-library/style.css	7.25 kB	+107 B (1%)
build/block-serialization-spec-parser/index.js	3.1 kB	+1 B
build/blocks/index.js	48.1 kB	-2 B (0%)
build/components/index.js	179 kB	+10 B (0%)
build/core-data/index.js	11.4 kB	-13 B (0%)
build/data/index.js	8.44 kB	+6 B (0%)
build/date/index.js	5.47 kB	-1 B
build/edit-navigation/index.js	4.05 kB	+513 B (12%)	⚠️
build/edit-post/index.js	28.1 kB	+540 B (1%)
build/edit-post/style-rtl.css	12.2 kB	+29 B (0%)
build/edit-post/style.css	12.2 kB	+30 B (0%)
build/edit-site/index.js	12.3 kB	+1.37 kB (11%)	⚠️
build/edit-site/style-rtl.css	5.19 kB	+88 B (1%)
build/edit-site/style.css	5.2 kB	+90 B (1%)
build/edit-widgets/index.js	8.33 kB	+833 B (10%)	⚠️
build/editor/index.js	44.3 kB	+708 B (1%)
build/editor/style-rtl.css	5.07 kB	+1.8 kB (35%)	🚨
build/editor/style.css	5.08 kB	+1.81 kB (35%)	🚨
build/escape-html/index.js	734 B	+1 B
build/format-library/index.js	7.63 kB	+2 B (0%)
build/keyboard-shortcuts/index.js	2.51 kB	+2 B (0%)
build/keycodes/index.js	1.94 kB	+1 B
build/list-reusable-blocks/index.js	3.13 kB	+1 B
build/media-utils/index.js	5.29 kB	-1 B
build/nux/index.js	3.4 kB	+1 B
build/plugins/index.js	2.56 kB	-114 B (4%)
build/primitives/index.js	1.5 kB	+2 B (0%)
build/rich-text/index.js	14.8 kB	+3 B (0%)
build/server-side-render/index.js	2.67 kB	-7 B (0%)
build/url/index.js	4.02 kB	-2 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
build/a11y/index.js	1.02 kB	0 B
build/annotations/index.js	3.62 kB	0 B
build/blob/index.js	620 B	0 B
build/block-directory/style-rtl.css	760 B	0 B
build/block-directory/style.css	761 B	0 B
build/block-library/theme-rtl.css	683 B	0 B
build/block-library/theme.css	685 B	0 B
build/block-serialization-default-parser/index.js	1.88 kB	0 B
build/components/style-rtl.css	16.9 kB	0 B
build/components/style.css	16.9 kB	0 B
build/compose/index.js	6.66 kB	0 B
build/data-controls/index.js	1.29 kB	0 B
build/deprecated/index.js	772 B	0 B
build/dom-ready/index.js	568 B	0 B
build/dom/index.js	3.1 kB	0 B
build/edit-navigation/style-rtl.css	485 B	0 B
build/edit-navigation/style.css	485 B	0 B
build/edit-widgets/style-rtl.css	4.67 kB	0 B
build/edit-widgets/style.css	4.66 kB	0 B
build/editor/editor-styles-rtl.css	428 B	0 B
build/editor/editor-styles.css	431 B	0 B
build/element/index.js	4.65 kB	0 B
build/format-library/style-rtl.css	502 B	0 B
build/format-library/style.css	502 B	0 B
build/hooks/index.js	2.13 kB	0 B
build/html-entities/index.js	622 B	0 B
build/i18n/index.js	3.56 kB	0 B
build/is-shallow-equal/index.js	710 B	0 B
build/list-reusable-blocks/style-rtl.css	226 B	0 B
build/list-reusable-blocks/style.css	226 B	0 B
build/notices/index.js	1.79 kB	0 B
build/nux/style-rtl.css	616 B	0 B
build/nux/style.css	613 B	0 B
build/priority-queue/index.js	789 B	0 B
build/redux-routine/index.js	2.85 kB	0 B
build/shortcode/index.js	1.7 kB	0 B
build/token-list/index.js	1.28 kB	0 B
build/viewport/index.js	1.84 kB	0 B
build/warning/index.js	1.14 kB	0 B
build/wordcount/index.js	1.18 kB	0 B

_{compressed-size-action}

[aduth]

I force-pushed a rebase to resolve conflicts introduced by #21921.

I'll plan to continue iterating on this tomorrow.

[aduth]

I pushed a few more commits here, experimenting with a couple extra ideas noted in the original pull request comment and from #21797 (comment), largely dealing with the "issue" of WP_Block constructor overhead and property consistency:

• WP_Block now implements a magic getter method to lazily initialize $block->attributes on-demand with defaults (688d4b7).
  • $block->attributes will consistently have default attributes prepared.
  • $block->attributes will not be initialized if it's never needed (i.e. if the block is static and thus never rendered via the dynamic render_callback logic flow).
• Inner blocks are lazily initialized as a WP_Block_List class which implements both Iterator and ArrayAccess (9f24221).
  • $block->inner_blocks members will only be initialized as WP_Block at a time at which where they are iterated upon.
  • Avoids array_map with closure in WP_Block constructor (see change).
  • Draws from alternate solution Block API: WP_Block implements JsonSerializable, Serializable, IteratorAggregate, Countable #21798 as reference.
  • Interoperability? It's not as concerning as Block API: WP_Block implements JsonSerializable, Serializable, IteratorAggregate, Countable #21798, in that we're not dealing with a value which already existed (the first argument $block of render_callback), but instead this $block->inner_blocks can be considered totally new, and not necessarily obligated to the same expectations of working with an array. It is "array-like".

To reiterate, both of these are optional enhancements, and can be assessed independently. Also, the WP_Block_List lacks some much-needed unit tests (in particular, I have 32.33̅% confidence the iterator behavior works correctly in all scenarios).

(cc @noisysocks, my new go-to anti-magic mentor 😄)

[epiqueras]

my new go-to anti-magic mentor

🤣

[aduth]

This is feeling quite stable now, and I'd like to let this sit in master a few days before next week's v8.1 plugin release, to shake out any potential bugs. I'll plan to merge this in the next few moments.

It's also a potential blocker for a related issue #22100.

[epiqueras]

Great work!

[noisysocks]

Sorry I was MIA! I appreciate the new title you've given me 😛

Yes, it's a little atypical in PHP to implement $block->attributes using __get. Many would prefer a $block->getAttributes() function instead. It's easy to find arguments about this if you search the web.

I think it's fine here and I like how it makes e.g. $block->name and $block->attributes consistent without sacrificing performance.

My only concern is that IDEs won't know about WP_Block::$attributes which means that developers won't get nice things like auto completion and type linting. I think we can address that by adding /** @property $attributes */ somewhere.

I like the WP_Block_List approach you took here and I think it's exactly the kind of use-case that Iterator is intended for.

[aduth]

Thanks @noisysocks ! And speaking as someone who has a tendency to comment on years-old pull requests, I'd be the last person to reject feedback after-the-fact 😄

Yes, it's a little atypical in PHP to implement $block->attributes using __get. Many would prefer a $block->getAttributes() function instead. It's easy to find arguments about this if you search the web.

I think it's fine here and I like how it makes e.g. $block->name and $block->attributes consistent without sacrificing performance.

For me, one of the other lesser objectives here was also to bring consistency between server-side and client-side interfaces, summarized earlier at #21467 (comment).

My only concern is that IDEs won't know about WP_Block::$attributes which means that developers won't get nice things like auto completion and type linting. I think we can address that by adding /** @property $attributes */ somewhere.

I was pretty disappointed about this as well. In fact, in early iterations, I kept the $attributes property descriptor on the class. However, as mentioned elsewhere, I didn't realize that __get will never trigger for a property which is defined on the class, even if it's unset. It appeared the only option to get the desired effect was to remove it altogether.

I wasn't familiar with the @property tag, but it does seem to be a perfect fit for how we're using the magic methods here. I don't see any mention of it in the relevant WordPress PHP documentation guidelines, though I don't see any mention to avoid it. To your point, it could be a good thing to include for IDEs which know how to interpret it.