Stop storing JSON blobs in XB config entities: improve DX

Clarified the goal.

Thank you so much for opening this. Translation from JSON to YAML and back seems hard, but incredibly worth it if the config entities can be treated as normal config entities.

I don't think this will affect config actions much, either way. They are severely limited in their ability to manipulate complex arrays, and regardless of how the data is stored at rest, we'll need a slew of custom config actions to make XB useful to recipes.

But it'd certainly improve DX to have the tree be a YAML structure. I personally don't see any reason why it needs to be a JSON blob, since YAML is a superset of JSON. I don't understand why it would be hard to move back and forth between JSON and YAML if we wanted to -- if you can decode some YAML to a PHP array, you can re-encode it as JSON.

I asked @effulgentsia if there was any particular reason why the XB trees are stored as JSON blobs. His answer on Slack:

I'm not aware of any good reason to keep them as JSON blobs. I think the only reason we didn't embed them as a proper structure within the config to begin with is just at that time not wanting to deal with config schema and validation. But now is the time to deal with that. In that issue.

I think that, to not break everything everywhere all at once, we might want to do this in multiple issues (first deal with inputs, then tree in another issue).

And I'm also thinking that, until 📌 Allow field types to control how properties are mapped to and from storage Needs work is done, we might want to have the ComponentTreeStructure and ComponentInputs data types handle both arrays and JSON blobs. That would ease the difficulty of the transition without blocking us, it looks like.

Okay, I made inputs a standard array -- does this look more or less correct? (I know that ignore is the wrong config schema type, but the tree and its inputs are validated as a whole, so I'm not sure it needs to be anything more specific).

A few questions, but this definitely looks to be on the right way! 👏

Next up:

• also tackling tree, not just inputs
• triggering a deprecation error for a JSON blobs for config entities (allows smooth upgrade, but ensures we as XB developers area ware)

Actually, hard disallowing seems fine given we're in alpha. Thoughts? :)

OK, I think I mostly have tree using regular arrays now, too.

The lone exception, and the reason the BC layer that converts between strings and arrays is needed, is that field items on content entities won't be storable as JSON transparently until 📌 Allow field types to control how properties are mapped to and from storage Needs work , so until then, we probably need to keep supporting strings of JSON. But we definitely don't seem to need those for storing trees in config.

I'm shocked by how simple this turned out to be 🤯🙃

Awesome work! 👏

Two things remain:

1. removing as many ::encodeXBData() calls as possible
2. #10's second bullet — which will reveal all ::encodeXBData() calls that MUST be removed

I know that this is kinda boring, but … it makes the entire codebase easier to work in, and keeps the entire codebase consistent. The MR in its current state removes some ::encodeXBData() calls, but on my first try I got tests to pass with more of those calls removed. So if this MR doesn't make things as consistent as they are in HEAD, it's going to be confusing for everybody else: "when do we need this ::encodeXBData() thing vs not?".

I've removed as many json_encode() calls as I could find. I don't think we should remove the BC layer yet because when the value is pulled from the database as a JSON string (when loading the fields), the data type plugins need to be able to handle that...right?

Update: FieldTypeUninstallValidatorTest now passes everywhere except on MariaDB. I imagine this is not a blocker, but worth calling out.

Bumping to per ✨ Create a schema for "allowed_html" which provides a better config diff Needs work .

As a byproduct, the TestDataUtilitiesTrait that @tedbow introduced in 📌 Convert test cases to use PHP arrays instead of JSON strings where possible Fixed to ease test development pain during initial development of XB, will largely be obsolete — that's what I asked for in #13, and which @phenaproxima almost completely finished! 🤩

In fact … since I'm feeling under the weather and my mind doesn't allow me to tackle anything complex right now, I just persevered and refactored it away: TestDataUtilitiesTrait is GONE! No more need to remember to wrap an array in a ::encodeXBData()! 🥳 All thanks to the one crucial insight by @phenaproxima! 👏

So: we'll not only make interacting with XB config less painful for site builders/XB users, but also for us developing XB, and hence improve development velocity slightly :)

P.S.: I'd still don't like that ->getValue(), TRUE has 7 matches in the codebase, but that's far better than the hundreds this MR removed. So, for the sake of iterative progress, didn't expand scope to that. Also: once 📌 Allow field types to control how properties are mapped to and from storage Needs work lands, XB won't have to work around that flaw in Drupal core anymore. If somebody wants to take that on: see attached PoC patch to get you going.

wim leers → credited larowlan → .

FYI: with this done, I think it's time to re-assess 📌 ServerClientConversionTrait Active and friends, aka make a decision on whether to keep ClientServerConversionTrait because it is good enough, or whether to transition to something else/better.