-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Server Side Rendering Parent Attributes Context #19685
Comments
We could also make things more explicit like React. The parent block can register attributes to provide as context: {
"name": "core/post",
"category": "layout",
"attributes": {
"postId": {
"type": "number",
"context": true
}
}
} Then children block can explicitly consume them: {
"name": "core/post-title",
"category": "layout",
"context": {
"postId": "core/post"
}
} |
Then the signature would be: render_block( $block['innerBlocks'][ $index++ ], $context ) |
The idea of explicitly registered context feels way more intuitive than merging attributes. I really like that API. |
It raises one question here, if this is a declarative API (block registration) (like the above), it could work for both frontend and backend in a transparent way. (I haven't made my mind yet whether it's the best approach. It comes with the cost that it's not flexible sometimes, so what are the limits: what If I want to set a context value that is not an attribute? |
When would that ever be a need? We need them to be "attributes" so that they persist between parses and serializations. |
I found some time to flesh this idea (#19685 (comment)) out, and I am thrilled with the results. I tried it with the Post, Post Title, and Post Content blocks. I also added support for an array of block types for consuming context from: {
"name": "core/post-title",
"category": "layout",
"context": {
"postId": [ "core/post", "some-plugin/fancy-post" ]
}
} The resolution will run left-to-right, only overriding previous values if set. Having this at the registration level is useful in Core for some complex blocks, but even more so for sharing parent and child blocks amongst Core and different plugins. The configuration can be filtered to both provide more/fewer attributes and to consume more/fewer attributes from more/fewer blocks.
I think this a significant step forward with our APIs. The benefits of this over current approaches are evident:
I think the last two are essential enough for me to believe this is the optimal direction. I am not sure how valuable entity providers will be if we go forward with this. Perhaps, they will be used outside of blocks, for other UI? If that's the case, we should still look at adding that |
Thanks for the deep dive.
Could you elaborate? In addition:
Is it on the table to not keep it?
I'm curious about the use cases for multiple sources for resolution. For two reasons:
This concerns me a bit. Aren't the two things reconcilable? Otherwise it sounds like we either got the entity-provider API all wrong, or we're now taking a direction that is too focused. |
With an function Parent( { attributes } ) {
return (
<EntityProvider
kind="postType"
type={ attributes.postType }
id={ attributes.postId }
>
<InnerBlocks />
</EntityProvider>
);
}
function Child() {
const prop = useEntityProp( 'postType', null, 'prop' );
return prop;
} It's not terrible, but if we need block context in the server, it'd be wasteful not also to use it in the client: function Parent() {
return <InnerBlocks />;
}
function Child( { context: { postType, postId } } ) {
const prop = useEntityProp( 'postType', postType, 'prop', postId );
return prop;
} Furthermore, what if we rely on some attribute unrelated to entities, would we introduce a different provider?
I don't think it will have any maintenance overhead, and it could be useful in non-block UIs, so I don't think we should deprecate it. But it's worth thinking about it.
That is possible with this approach. I think you mixed things up here a bit. Here is an excellent practical example: Newspack implements a Homepage Article block, which functions like the Core Post block but has a lot more features specific to their clients. They'd like to reuse some of the Post field blocks like Post Title without having to clone them and pollute the inserter. They could filter its registration and change
The entity provider API is excellent and could continue to be valuable for building entity based UIs other than blocks. The issue with it in the context of block authoring is that there's no easy way to map it to the server rendering process. In the process of trying to find a solution for this, I realized that a more general block context could work for any attribute, not just entity API parameters and that the API can work for both server and client. |
Oh, I see what you mean now. The Newspack example also explains this. Forcing them to add a Core Post block inside their Homepage Article block would clutter the UI so we would probably need some concept of invisible blocks, and what if they want to position |
Yes, this is it. :)
Definitely, that wouldn't be ideal. But it still feels like allowing several block types to provide context to be the wrong workaround. What if Homepage Article could just borrow something from Core Post to achieve the same result semantically? Off the top of my head, This may also encourage third-party blocks to follow the conventions and semantics of their closest core equivalents, and perhaps act as expansions thereof — rather than (more often than not incompatible) substitutes, which is easier to get away with if you can just add a new block type to the child block's context requirements. |
Interesting. It would have to be something like: {
"name": "core/post-content",
"category": "layout",
"contextName": "core/post"
} So that it also works for server-side rendering. I don't see how this will encourage the following of conventions or semantics. They can just as easily overwrite the context and go down the wrong path. But, a benefit I do see with this is that it's simpler than filtering every post field block. There a quite a few limitations to this, though. It removes the possibility of them building blocks that only consume from What if two post providing blocks share some consuming blocks and also have some that they don't share. You want to introduce a third post providing block that uses all of the consuming blocks. There are also possibilities for breakage. If there are already two post providing blocks sharing consuming blocks and a plugin wants to introduce an extra consuming block that only works with 1, there would be no way of doing this without completely rewriting all the other block configurations. I guess all of these are symptoms of the fact that if you allow blocks to share a providing context, you create dependencies on all current and future consuming blocks. My proposal doesn't have this issue because each block gets its context, and adding providers to consuming blocks doesn't implicitly create dependencies from future providing blocks and doesn't affect other consuming blocks. |
If you are to expand on an established core-defined shape, then breaking that convention is something that can be called out as wrong. In contrast, if you're free to build an entirely different post-provider, you have all that freedom and flexibility, at the cost of framework-level commonalities. I don't know how significant its impact will be in practice, but the difference exists. ¯_(ツ)_/¯
What if it didn't? Initially I was going to propose an inheritance-type model in that a
That way, the onus isn't on the child to state all possible providers of post context. Instead, providers need to be clear about their data. They aren't forced to conform to core patterns and can diverge, but they are naturally encouraged to follow core patterns if they want to be adopted by users who want the ability to mix together parents and children from different block libraries (core and multiple third parties).
Yeah, this seems very related to what I just described. :)
For sure. We need to think this through.
👍 |
I'm coming to this a bit late. That said, the work here and in #19572 is looking good. Both the implementation and the use-cases ring reminiscent of React's context feature, which I'm sure is not unintentional. I have a few initial observations:
I'm interested in this conversation about how a child is expected to declare their context needs. My first impression was that we don't need to be so strict about how and from where this context is assumed to come. If a title block needs a post ID, why can't it just declare as such: {
"name": "core/post-title",
"context": [ "postId" ]
} Aside: I expect we'd need or want to support multiple context values being consumed in a block, so being able to express it as an array (if even optionally) I feel would be wise. The way I picture it is that it acts the same as React context, in that it will use the value of the closest ancestor block which provides an attribute context by that name. There could be multiple ancestors, or just one, or maybe even zero. The closest "wins". To the question of: From where specifically is I guess there could be a potential concern for conflict, moreso in cases where a key could be ambiguous in how it's expected to be used. A context of
Considering again the similarities between this and React's context: In React,
As noted previously, there's a strong likelihood that the context that an ancestor provides will correspond directly with one of its own attributes. I raise this not because I necessarily see it as a problem, but moreso just to highlight the inconsistency, because I could foresee this becoming a point of confusion. There's also a question of: Is there ever a case where an ancestor might want to provide some context which isn't precisely the value of one of its own attributes? Such as:
A problem we'd likely encounter going down this route is that it's fairly straight-forward to compute a context from attributes consistently between server and client. To implement a derived or ad hoc value which can work both in the server and client is not nearly as clear, and would probably require the block developer to implement it separately in both places. We could still define context as a top-level property of the block, mapping from context to attribute name: {
"name": "core/post",
"attributes": {
"postId": {
"type": "number"
}
},
"context": {
"postId": "postId"
}
} There's pros and cons to this. It definitely feels redundant. But on the other hand, it elevates context to a separate and top-level property of a block type, reintroduces consistency of context being a separate concept from attributes, and leaves open some potential future of additional mapping sources.
With this, I am mostly talking about the server-side d968c0c#diff-6ff32417da0658502e7caa1a1abbeae6R203 ... where the function render_callback( $attributes, $block_content, $block ) {} It's the same as what was proposed with Trac#48104, and in many ways similar to the problems discussed in #19401. The problem then and now is that there's high redundancy here, in that The root of the problem is that we should have always just given Our options as they exist today are:
|
You used the same key for the producer and the consumer, may need a suffix.
There's another approach maybe less clean but common in WordPress, using a global variable you can access inside the render callback. I'm also wondering whether the consumer need to declare anything in its setting or just pick from the context bucket as needed. Thinking more here I do like the declarative approach as it opens the door for smart behaviors in the inserter (show/hide blocks depending on context...)
I like this approach. 🤷♂ |
I'm not sure I follow? They're the same because the consumer is consuming the key as provided from the provider. Edit: I saw there was a typo in that I was using
😱 ... but yes, technically it is an option 😄 There is prior art in |
What If a block wants to be a provider for something and a consumer for something else. |
Oh! You're right. I conflated the
|
On the point of redundancy, I think it could be simplified further if (at least for an initial implementation) we assume a mapping to attributes: {
"name": "core/post",
"attributes": {
"postId": {
"type": "number"
}
},
"providesContext": [ "postId" ]
} This still supports expansion to support additional sources. For example, overloading such that the following expressions could potentially all be equivalent: Implicitly from attributes, retaining attribute naming: { "providesContext": [ "postId" ] } Implicitly from attributes, customizing context naming: { "providesContext": { "postId": "postId" } } Explicitly from attributes, optionally customizing context naming: { "providesContext": { "postId": { "source": "attribute", "name" "postId" } } } Considering again the need or feasibility of other sources of context (#19685 (comment)), I don't know that strictly-speaking these values would need to be serialized, at least in how they're evaluated server-side. I do think it may (needlessly?) complicate how this would be implemented universally, where if we can assume in both the server and client to source from a known set of values (attributes), the framework can easily handle how those values are sourced. To some extent, there are parallels between this and comment-based attributes storage vs. attribute sources (which are still known by the framework how to resolve... well, at least client-side). I do think it would be important to find at least one compelling use-case before investing too much in this idea that we'd need additional sources of context values. But even if considered on its own merits as an alternative to embedding a
|
It's arguable whether the descendent should need to declare its own context needs. The need for it seems to come from technical limitations of working with React to be able to subscribe to changes in that context. Server-side, at least as considered to be implemented in #19572, it's merely used to know how to pick values from a shared set of context values. At a high level, I don't know that I would have any strong reservations against simply making that shared set available in its entirety. But those React limitations being what they are, at least if we assume that the context values can change over the lifetime of the editor, I don't know that we have any option but to require it. And granted, it may bring some benefit to have the option to inspect a block type to understand what it is expecting to be provided as context. |
Considering #19685 (comment) :
From the perspective of the public-facing Also, considering the scenario we're running in to with I see a couple options here:
Pseudo-code: function render_block( $block ) {
global $block_context;
$block_context_before = $block_context;
$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
foreach ( $block_type->providesContext as $attribute_name ) {
$block_context[ $attribute_name ] = $block['attrs'][ $attribute_name ];
}
$block_content = '';
foreach ( $block['innerContent'] as $chunk ) {
$block_content .= render_block( $block['innerBlocks'][ $index++ ] );
}
$block_context = $block_context_before;
return $block_content;
} |
I think the recent bug fix for The biggest limitation of the proposed approach is that we use the name of the attribute as the name of the context, so when you have a few levels deep nesting, if you want to expose |
@gziolo I think there are cases where we'd want to make it easy to intentionally override the value based on a name, but to your general point, I agree there is some potential for unintended conflict. From my earlier comment:
Is this the problem you had in mind? Do you have a use-case you can imagine where a deeply nested block would need to be aware of the post ID not of its parent, but of its grandparent? Is it something where you think we'd want a system of namespacing, or only that we'd need to be careful and intentional in the context names we choose? |
Yes, it's very similar. I guess @youknowriad or @mcsf could know best whether it's a valid concern. I don't quite know how it all exactly works at the moment, but I would prefer we confirm if it for template parts and post content upfront. Just thinking only about that, I can see that the page has type and id assigned, then template part has internally another type and id, inside you might want to use let's say block that would read type and id from the parent, now the question is if it can read the type and id for the page or it's overridden by the template part? It might be a non-issue and I don't understand it all fully. Someone could clarify this cascade and relations :) |
I'm not sure yet if I'm understanding your terminology exactly (I'm still learning Gutenberg's in and outs) but I have different headings for pages depending on what their grandparent is (e.g. display 'democracy' header and its navigation menu on a page slug of site-name.org/democracy-2020/events/speaker-series... The speaker-series page is a grandchild descendant of democracy-2020); sorry if I totally misunderstand. (The pre-Gutenberg logic of this is in our theme at https://gitlab.com/cpl/tempera-nocopyrt/-/blob/a1afc094fa51f1a3f06e69cd650e016b02028f91/header.php#L60 ) |
@skorasaurus It does seem related. It's not clear based on how it's currently implemented if it might be solved differently in a fully block-based theme template. And if it would use the context idea explored here, whether it may be implemented using a different key to identify the post of the relevant ancestor. Otherwise, I think it would require that we allow access not only to the closest context provider which provides a value for that key, but instead the full chain of ancestors who do. I worry this may add some complexity, especially if there may be different solutions to this problem. It's a good use-case to consider, though. |
The Problem
A lot of patterns have surfaced where we need inner blocks to be aware of specific values configured on their parents.
For example, the Post block needs to set a post ID context value for a Post Title block child.
This introspection is hard to do because rendering happens depth-first, so child render-callbacks run before their parents run theirs. Blocks like the navigation block get around this by rendering their children themselves.
Imagine having a 3 level pyramid of blocks implementing this pattern:
The callbacks would run in the following order:
The
+
signs represent the extra re-renders caused by this pattern. The approach converted this linear computation into something that scales with the square of the number of levels times the average number of children per node?A Solution
I propose an alternative way for parents to provide data to their children during server-side rendering. It would require minimal changes to Core, and it fits in nicely with our heavy client-side usage of React Context.
Basically, we should extend
render_block
so that it's last parameter is a union of its parents' attributes:This approach would allow blocks like Post Title to check for the nearest parent provided post ID easily, and the Navigation Link block to access the provided class name and style attributes so that they can render themselves correctly.
The text was updated successfully, but these errors were encountered: