Fix: Properly render <template> capability heading without spacing wo…#250
Fix: Properly render <template> capability heading without spacing wo…#250Abhikorekar wants to merge 1 commit intoProjectEvergreen:masterfrom
Conversation
thescientist13
left a comment
thescientist13
left a comment
There was a problem hiding this comment.
Thanks for giving this a go @Abhikorekar !
Took some time to review and I think the issue is a bit more fundamental, and based on my initial research based on your changes here, the root cause I think is within WCC itself (technically parse5).
On top of that, when building locally (or checking out the deploy preview) the content is not rendered correctly.
I think the way forward is to explore parse5 a bit more and see if there is something we can do there when parsing the content's the HTML of a user's component to better handle text vs tags?
Let me know your thoughts.
(I'll also update the original issue based on my findings)
| "remark-github": "^10.0.1", | ||
| "stylelint": "^16.10.0", | ||
| "stylelint-config-recommended": "^14.0.1", | ||
| "ts-node": "^10.9.2", |
There was a problem hiding this comment.
not sure why we're adding ts-node? I don't think it should be needed here, unless I am missing something.
| <wcc-capability-box heading="< template >"> | ||
| <p>Template element support (createElement)</p> | ||
| </wcc-capability-box> | ||
| <wcc-capability-box heading="<template>"> |
There was a problem hiding this comment.
looks like formatting is off here. Prettier formatting should catch this, if you run it before committing
| {innerHTML} | ||
| </div> | ||
| ); | ||
| const heading = this.getAttribute('heading') ?? ''; |
There was a problem hiding this comment.
Per your comment in the description
Updated wcc-capability-box to render heading using textContent instead of JSX interpolation.
Could you could expand on your solution here? I'm wondering if this is something that could / should be addressed within the JSX transformation itself. If you can provide some more details on the issue you observed, perhaps I can guide us to a solution that doesn't requiring refactoring the entire component.
Maybe it's a parse5 issue? 🤔
There was a problem hiding this comment.
Yeah, doing a little playing around with AST explorer and browsing the parse5 GitHub, it does seem to be a parse5 "issue" - inikulin/parse5#191
In AST explorer, i can see <template> is getting parsed as <template> literally
So I think the real work is not in the component itself, but rather in WCC itself, likely here
https://github.com/ProjectEvergreen/wcc/blob/0.19.0/src/wcc.js#L25
There was a problem hiding this comment.
Thanks for confirming the parse5 behavior.
After stepping back from the component-level workaround, I agree this doesn’t seem like a rendering issue inside wcc-capability-box, but rather how WCC handles parsing and serialization after parse5 decodes entities like <template> into .
Looking at src/wcc.js, we parse via the DOM shim (which uses parse5) and later serialize the transformed tree with serialize(). Since parse5 decodes entities per spec, the attribute value becomes in the AST, and it appears to be getting interpreted as markup somewhere in that flow.
Before I make further changes, would you recommend focusing on how attribute values are handled during serialization (e.g. before serialize(finalTree)), or is it better to adjust behavior in the DOM shim layer?
I just want to make sure I’m addressing this at the correct abstraction level in WCC rather than introducing another workaround.
There was a problem hiding this comment.
Before I make further changes, would you recommend focusing on how attribute values are handled during serialization (e.g. before serialize(finalTree)), or is it better to adjust behavior in the DOM shim layer?
Hmm... that's a good question. My hunch is that the bulk of the parse5 work is happening in the DOM shim, but I don't know if there would be any sort of pre-processing needed from WCC first to understand of there are entities that might get transformed? Since the behavior seems inherent in parse5, as the linked issue suggests, maybe some handling needs to happen on the way in and out? 🤔
Honestly, what I would do personally in a situation like this is create a unit test for the behavior in the WCC test suite (and with #246 we can do the same for the capability box component itself) and I would just explore what's possible to get the desired outcome with a lot of logging and a very narrow test case. Since I'm not sure to what degree working around parse5 would entail, and without a clear understanding of the exact solution, I would probably worry first about what it would take just to get things to work, and sort it out once I've made it that far.
Probably not a great answer, but that's pretty much how it goes sometimes. So maybe start a new branch / PR there? Happy to assist and answer questions as you make progress 🙌
2 participants
…rkaround
Related Issue
Closes #249
Summary of Changes
<template>capability heading rendering without spacing workaround.<template>to prevent SSR parsing issues.wcc-capability-boxto render heading usingtextContentinstead of JSX interpolation.