SSR: Handle closing tags whose openers had an attribute directive#144
Conversation
aacd1e1 to
3d4a576
Compare
83d037a to
b2c9da5
Compare
b2c9da5 to
f9eb75c
Compare
luisherranz
left a comment
There was a problem hiding this comment.
The code looks fine to me 🙂
Although I thought the closing tag logic was going to be delegated to the HTML (Tag) Processor. Is it not the case anymore, or is this a temporary solution until something like this is finished?
Thank you! 🥳
I'm not sure at this point. We still need logic in the HTML (Tag) Processor to handle closing tags, in order to implement methods such as WordPress/gutenberg#47573 has logic that's somewhat similar to what we're doing here (stack of opening tags). However, I didn't find it directly applicable to what we're doing here, and so I opted to implement it separately. I wouldn't rule out that eventually, we will have logic in the HTML (Tag) Processor that can serve both purposes, but they seemed different enough that I didn't see any harm in implementing this one separately already (rather than waiting for the HTML (Tag) Processor). It's also possible that the purposes are different enough that we'll continue to handle things differently. (After all, we can optimize the logic here a bit to e.g only track opening tags of a given type 🤷♂️ ) cc/ @dmsnell 😊 |
the thing that's been stewing lately and I hope to still get into soon is that I think we can avoid creating this and instead only expose the expected function, this requires taking some time to understand the adoption agency algorithm, but from what I can tell it's mostly adding a few tag-type lists, maintaining a small tag stack (not unlike here and in our other HTML processing PRs), and crossing off all the cases of format boundaries. this isn't to speak to this PR; go crazy here in your own code. for the HTML API I think that given that it does actually seem possible to do things 100% I'm tempted to avoid introducing things that are like the 100% but aren't. |
Some attribute directives (such as
wp-context, see #143), need to be run not only on an opening tag (e.g.<div wp-context="...">but also on the matching closing one. (Forwp-context, this is required to reset the context to the value that corresponds to the enclosing scope's context.)This needs somewhat complex logic to keep track of nested tags. Consider e.g. the following example (used in this PR's unit test):
Here, we want to run the
foo-testattribute directive processor on the closing</div>that matches the opening<div foo-test="abc">-- but not on any of the other two closing</div>s.Rather than requiring individual directive processors to implement the required tracking of nested tags, we implement it in the
wp_process_directivesfunction.To that end, this PR also moves it into a separate file and changes its signature a bit for better testability. Finally, we're adding a unit test to verify the behavior described above.