Centralize & standardize logic for constructing *PropSource objects + kernel test coverage

Wim Leers → credited larowlan → .

Blocked on ✨ [MR Only] Edit any component prop, powered by a new FieldForComponentSuggester service, which will power the JS UI Fixed landing first, but @larowlan can already review this — just only review the last commit 🙏

✨ [MR Only] Edit any component prop, powered by a new FieldForComponentSuggester service, which will power the JS UI Fixed is in, rebased, ready for review 👍

The MR pipeline is failing.

Pipeline is passing

@larowlan or @tedbow, could you review? 🙏

@larowlan Thanks! Merging in all upstream commits first…

Only developers would maybe see the string representation of (Static|Dynamic)PropSources. And they definitely won't be reading that representation in JSON-encoded-in-YAML form (although there we could do the work to not use \u<unicode code point> but just use the Unicode representation directly — we should do that, it's just not urgent, especially if we don't know we'll continue to use this approach).

This is a data structure ((Static|Dynamic|Adapted)PropSource are strongly typed PHP objects), it's just one that has an equivalent string representation. That string representation allows both for a more compact representation, resulting in less (cognitive & space) overhead when looking at an entire component tree. I wanted a string representation
because that makes searching (googling/ducking) for problems much simpler.

I respectfully disagree with you that only a data structure, without a string representation, would be better. You'd then at minimum still need an array representation to be able to express/store it in JSON/YAML, and I wanted to avoid a new Type Of Array of Doom 😅 — but it sounds like you think this is perhaps the first Type of String of Doom? 😅

All that being said: this seemed the best/simplest path to something that works that I could see. If you feel so strongly that there is a better way, then I'm absolutely open to an alternative. Whether you provide that in the form of a photo of drawings on a napkin, sample JSON blobs or an MR, is your choice :)

Assuming that the \u<unicode code point> aspect was a significant part of what @larowlan was referring to in his MR comment, that's easily solved by using https://www.php.net/manual/en/function.json-encode.php.

Note that \Drupal\experience_builder\PropSource\StaticPropSource::__toString() already uses that … it's just that https://git.drupalcode.org/project/experience_builder/-/blame/0.x/config... was handcrafted and does not yet have validation constraints that would detect/prevent this particular encoding. Fixed with a simple print json_encode(json_decode($s), JSON_UNESCAPED_UNICODE);: https://git.drupalcode.org/project/experience_builder/-/merge_requests/4... :)

Add test coverage to A) reduce the concerns voiced by @larowlan, B) empower him/others to propose an alternative implementation. The test shows the range of needed capabilities — EndToEndDemoIntegrationTest already did that, but the new PropSourceTest does so in more detail, and in a kernel instead of functional test: https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...

@larowlan wrote but did not click "approve" — so bypassed the explicit approval 🦹

Re #11 @wim - my driver is making sure we don't repeat mistakes like https://dbushell.com/2024/05/07/modern-wordpress-themes-yikes/

In last week's meeting longwave voiced similar concerns about this and asked if JSON path was considered as a non dtupalism approach

😄 I sent that exact link to @lauriii last week, voicing the exact same concern!

Related: 📌 Document the current JSON-based data model, and describe in an ADR Active

Automatically closed - issue fixed for 2 weeks with no activity.