Closed (fixed)
Project:
Custom Elements
Version:
3.x-dev
Component:
Code
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
12 Sep 2024 at 16:39 UTC
Updated:
10 Feb 2025 at 19:59 UTC
Jump to comment: Most recent
See parent task.
TODO ON ME: check if tests for JSON serialization can be easily added to this module or if e.g. lupus_ce_renderer is a better place.
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
fagoI second that, the parent issue also triggered that thinking for me.
We should make one test that covers all cases that are relevant for serialiation, what I can think of is:
CE with attributes, no children
CE with attributes, single slot, single CE child
CE with attributes, single slot, multiple CE children
CE with attributes, single slot, markup cotnent
CE with attributes, single slot with multiple entries, one single child, one multiple CE children. adding markup in this case is unusual, so not something we have to cover imo.
then, we should serialize this in those formats:
markup with vue2/standard CE syntax
markup with vue3 syntax
json
This is not a release blocker, but I'd love to get it done soon to ensure we are good and stay good here. We should make this one or multiple dedicated kernel tests, so we can easily add them to 2.x and 3.x and compare results.
Comment #3
roderikFor reference to whomever:
Markup:
SeerenderCustomElement()in existing tests.JSON:
$output = $this->getCustomElementNormalizer()->normalize($custom_element). (There is no place in custom_elements module that does JSON serialization, but the building blocks - i.e.getCustomElementNormalizer()are here so the test should be here. One module that has the above line of code, is lupus_ce_renderer.)Comment #6
benellefimostfa commented@roderik First iteration of the test is pushed, which is failing when having multiple slots after normalizing.
Comment #7
roderikThis looks good, and I also learned things about CE in the process. (The outdated issue summary is a sign of how little I knew about CustomElement basics back in September -- I likely won't bother updating it.)
I have some requests for additions though; see MR comments.
Please change ticket status to "Needs review" when I should review.
Also: I will want to have a larger comment on the class (to say that it is not only testing CustomElements but also rendering) and possibly rename it. But I think I'll just do that myself afterwards, and not bother you with that.
I actually think we'll need to cover this because it will become more important. But we can add that coverage in #3494426: Create markup to custom-elements tree parser after merging this.
Comment #8
roderikReviewed. This is good. But done:
I don't usually merge issues that are still set to Needs Work, but review was requested elsewhere and we can continue in #3494426: Create markup to custom-elements tree parser if needed.
Comment #10
roderik