Tracking Issue: Server-side rendering of directives

[ockham]

Based on #118 (comment) and #125.

✅ - Implemented
🟡 - Waiting for an upcoming Gutenberg release to include a missing feature
❌ - Blocked by missing feature in WP_HTML_Tag_Processor

Name	Directive	Missing feature
wp-context	✅ (#143)
wp-show	🚧 (#141)	Wrapping content inside HTML tags with extra tags. E.g. get_content_inside_balanced_tags() from WordPress/gutenberg#46345, and a matching set_content_inside_balanced_tags() (see WordPress/gutenberg#47036).
wp-bind	✅ (#133)	get_attribute_names_with_prefix(); landed in GB 15.0
wp-class	✅ (#133)	See wp-bind.
wp-style	✅ (#133)	See wp-bind. Furthermore, see WordPress/gutenberg#46887. (The latter isn't a blocker, though.)
wp-text	✅ (#170)	Replace content between HTML tags. See wp-show.
wp-for / wp-each	🚧 (#166)
wp-html	✅ (#170)	Replace content between HTML tags. See wp-show.
wp-slot & wp-fill

TODO:

• Properly parse expressions in attributes like when.
  • Includes nested JS-style objects (e.g. context.myblock.open).
  • Basic logic operations (negation, and, or)? Comparison?
• Add some directive/components registration mechanism.
  • Maybe similar to blocks, patterns, etc? register_directive, takes a directory with a directive.json which includes information such as attributes, render, context, ...?

[ockham]

I’ll stop working on the server-side rendering of directives for now since we’re shifting our focus to other priorities. Overall, the Tag Processor has proven to be a good fit for this, and we’ve successfully implemented SSR for a number of directives. We’re still having some issues with wp-show and wp-each, but we’re confident that they aren’t insurmountable.

[ockham]

wp-show

wp-show is more complex than the directives we’ve implemented so far as we want it to modify the outer HTML of the tag that the directive attribute is attached to (to wrap that tag in a <template>). In #141, I’ve tried to implement a wrap_in_tag() method, which should be a bit simpler than a generic set_outer_html.

When testing #141 against the Movies Demo repo, it was discovered that it didn’t return the correct HTML. While I suspected the issue was related to the internal state of the Tag Processor, I thought I managed to find a workaround in #215. Unfortunately, I found that there was yet another issue that it didn’t fix.

The problem is exacerbated by the fact that those issues are hard to reproduce and isolate outside of the "real-world" environment where they’re documented (the Movies Demo, in this case). This is likely due to the state of the Tag Processor being set in a specific way by other previously processed directives, or even the sequence of calls to methods such as next_tag(), get_updated_html(), or seek().

I managed to isolate at least one failing test and carried it over to the wordpress-develop repo to allow us to fix the issue there. @dmsnell, who is more knowledgeable about the Tag Processor, has been so kind as to help me investigate. He managed to reduce that test case further and filed a tentative PR to fix it.

Our next steps should thus be to review and test the fix, and to apply it locally to see if it fixes the issues we were having in #141; specifically:

• SSR: Add wp-show attribute directive processor #141 (comment) and
• SSR: Fix directive processor invocation #215 (comment)

I’ll to try to dedicate some time to this in the next couple of days but will start to shift my focus to other priorities.

[ockham]

wp-each

Example:

<table>
    <tbody data-wp-each:item="context.data" wp-key="id">
        <tr>
            <td class="col-md-1" data-wp-text="context.item.id"></td>
            <td class="col-md-4">
                 <a wp-on:click="actions.select" data-wp-text="context.item.label"></a>
            </td>
        </tr>
    </tbody>
</table>

where context.data is array with multiple entries. In this example, each entry is an associative array with an id and a label key.

Processing wp-each then involves

• setting some sort of context value for the current loop item, and then
• processing any directives found in the markup that's wrapped inside the tag with the wp-each directive (the tbody in this example).

The problem is that this processing of directives needs to happen repeatedly, i.e. once for each loop item.

This isn't compatible with the way we're currently processing directives (in wp_process_directives), where we iterate over all tags, and stop to process any directive we find along the way by invoking its directive processor. What's missing is some way to:

1. either allow wp-each to process content recursively, so that it can be implemented by a simple foreach loop over all loop values, and by calling wp_context_directives() on its nested markup each time, with loop item context set appropriately,
2. or to "deconstruct" the loop,
   1. e.g. by unfurling the nested markup, i.e. wrapping the nested markup in a "virtual"/"temporary" wp-context (that's removed before sending content to the client) for each loop item, or by
   2. going back to the tag opener with the wp-each directive after processing the nested markup, to set the item context to the next value, and process the nested markup again.

Recursive approaches

We have a proof-of-concept PR for option 1: #166. However, I don't like that we have to pass the list of directive processors to the individual wp-each processor (in order to be able to recursively call wp_process_directives), as it goes against separation of concerns.

An alternative could be to absorb the list of directive processors into the WP_Directives_Processor object -- it will then be fairly opaque to each individual directive processor, and we might be able to add some methods to limit it to the markup that's nested inside the tag with the wp-each directive, so that we can use it for the recursion.

I've started experimenting with absorbing the recursion into the outer control flow, i.e. wp_process_directives (#209). However, I'm not sure that that approach will actually work with directives like wp-each which need to call wp_process_directives themselves. It seems like we would find ourselves again in a situation where, in order to rely on the "outer" control flow to take care of the recursion, we'd be facing the same problems as with the iterative approaches.

Iterative approaches

The "implicit" approach (option 2.i) seems riddled with problems, as we'd need to keep track of which loop items we have already processed, and we'd need to store the markup for the loop items that we've already processed. I never got far in my experiments as that seemed very unwieldy.

This makes the "explicit" approach (option 2.ii) -- where we actually copy the wrapped markup n times (once for each loop item), and wrap each copy in a wp-context directive -- look less bad. Open questions include:

• How do we mark a directive as server-side only/"temporary", so we remove it before sending markup to the client?
• What tag would we use for each loop item's wp-context directive?
• How do we retain the original wrapped markup (to send it to the client)? (Do we need to?)

---

I realize that these musings might be a bit more abstract; unlike wp-show, I haven't written that much code to explore these different avenues yet. Feel free to get in touch if you'd like to work on this and need a bit more context or explanations.

[dmsnell]

I don't like that we have to pass the list of directive processors to the individual wp-each processor (in order to be able to recursively call wp_process_directives), as it goes against separation of concerns.

I'm not sure I see this as violating any separation of concern issues, particularly since a directive handler might want to add or remove from that list (i.e. remove itself).

Still, this is not a concern that has to be exposed. It would be a choice to make, but it's just as easy to pass the recursion into the function itself as a closure encapsulating any general state.

$recurse = function( $tags, $prefix, $context ) use ( $directives ) {
	return wp_process_directives( $tags, $prefix, $directives, $context );
};

process_wp_each( $tags, $context, $recurse );

function process_wp_each( $tags, $context, $process_recursively ) {
	…

	$inner_html       = $tags->get_inner_html();
	$inner_tags       = new WP_Directive_Processor( $inner_html );
	$inner_tags       = $process_recursively( $inner_tags, 'data-wp-', $context );
	$loop_inner_html .= $inner_tags->get_updated_html();
	$context->rewind_context();

	…
}

The "implicit" approach (option 2.i) seems riddled with problems, as we'd need to keep track of which loop items we have already processed, and we'd need to store the markup for the loop items that we've already processed.

I'm probably missing something, but this doesn't seem like it'd require too much state tracking. Would be curious where you see this causing more hassle specifically.

copy the wrapped markup n times (once for each loop item), and wrap each copy in a wp-context directive

Hm. That seems clever 🤔

[ockham]

I don't like that we have to pass the list of directive processors to the individual wp-each processor (in order to be able to recursively call wp_process_directives), as it goes against separation of concerns.

I'm not sure I see this as violating any separation of concern issues, particularly since a directive handler might want to add or remove from that list (i.e. remove itself).

I think I'd rather isolate them from each other (and generally isolate the directives from the processing loop or recursion). I don't see the immediate need to allow them to add or remove other handlers (or even themselves) to or from that list, respectively, so I'd prefer to go with YAGNI 😬

Still, this is not a concern that has to be exposed. It would be a choice to make, but it's just as easy to pass the recursion into the function itself as a closure encapsulating any general state.

That's fair, thank you for bringing that up!

The "implicit" approach (option 2.i) seems riddled with problems, as we'd need to keep track of which loop items we have already processed, and we'd need to store the markup for the loop items that we've already processed.

I'm probably missing something, but this doesn't seem like it'd require too much state tracking. Would be curious where you see this causing more hassle specifically.

I meant option 2.ii (not 2.i) -- apologies if that caused any confusion:

going back to the tag opener with the wp-each directive after processing the nested markup, to set the item context to the next value, and process the nested markup again.

As for the problem with 2.ii, I'll try to explain in pseudo code. Let's go with the example markup from above. Here's a naive first iteration:

1. We encounter data-wp-each:item="context.data" and thus call process_wp_each( $tags, $context ).
2. We read context.data. We set context.item to its first item.
3. We return from process_wp_each and pass control back to the overarching loop (wp_process_directives).
4. wp_process_directives continues evaluating the markup (with context.item set appropriately), until it encounters the closing tag that matches the opener that had the data-wp-each directive. So far, so good: We've successfully processed the enclosed markup with context.item set to the first item from context.data 🎉

But this is where it gets tricky. We can now jump back to the opening tag, but we'll need to make sure that next time we set context.item, it will be the (n+1)st item from context.data. So maybe at step 2, we should've stored the number n somewhere in context. Or even better maybe, the entire context.data array (so we don't have to re-evaluate the directive attribute) -- and coming back to it now, we might just array_shift() it.

But there's another, bigger, problem: We've already processed the markup that's enclosed in the tag with the data-wp-each attribute and its matching closer -- but now, for the n+1st iteration, we once again need the original, unprocessed markup. So we should've kept the original markup somehow. Two options come to mind:

• Keep it in context, and once we reach the tag closer, store the processed markup from the last iteration somewhere (hello context my old friend). Then, overwrite the enclosed (processed) markup in with the unprocessed markup so we can jump back to the opening tag and process it now with the n+1st item. (This means that once we've gone through all items, we need to replace the enclosed markup with the concatenated, processed markup from all iterations.) Doable but... damn, that's a lot of state to keep track of.
• Don't overwrite the enclosed markup in the first place; instead, keep the unprocessed markup, and write the processed markup to... somewhere else. But that means we basically have to make a copy of the unprocessed markup and run a separate Directive Processor on it -- in short, it's equivalent to the recursive approach.

copy the wrapped markup n times (once for each loop item), and wrap each copy in a wp-context directive

Hm. That seems clever 🤔

Thank you! Not sure I can take credit for it, I think it was @luisherranz' idea 😆

Since it makes things more explicit and easier to reason about than what I described above, it's quickly becoming my preferred approach.