Vue-incompatible default output for text/text_long type fields

Created on 20 January 2025, about 1 month ago

Problem/Motivation

The default processor logic for "text" / "text_long" field types produces a custom element that is normalized to JSON like:

"content": {
  "element": "node-article-full",
  "body": [
    "<p>body</p>"
  ],

This array structure is not valid for a slot (at least for Vue components?).

Note: that's formatted text without summary.

"string" types == plain text fields just put their values in an attribute.

Text with summary produces

"content": {
  "element": "node-article-full",
  "body": {
    "element": "field-text-with-summary",
    "body": "<p>body</p>",
    "bodySummary": "<p>summary</p>"
  },

...which is valid.

Steps to reproduce

Look at the standard API output for an article, which has a body field.

Proposed resolution

There's no real reason that a single-value body field should produce an array. We can prevent that happening while building the custom element in DefaultFieldItemListProcessor.

(Alternative discussed in comment. after I create the MR.)

Remaining tasks

  • Review MR for direction; maybe briefly think about alternative?
  • Write tests, after direction is approved.

API changes

This will change default JSON output for any single-value field that results in setting a single named slot with a single value (i.e. that does just a singular setSlot() call).

Note: I've tested content_format="markup" manually; it's not affected by the proposed change.

  • Needs change record.
  • Needs approval to commit this change to v3.1.

No matter how we implement this: If we want Custom Elements to have sensible defaults, then those sensible defaults will be different from (i.e. a 'compatibility break with') the current default output.

So we could make this into e.g. a different formatter for only text fields instead, but

  • the default output would still need to change - to that new formatter
  • I am not aware of other field types that would be negatively affected by this change. That would be custom code/processors for a single-value field that do a single setSlot() call.
🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @roderik
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • Merge request !111Change output for single-value text fields → (Open) created by roderik
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    The phpunit failure is not for this change; it's only depecation messages which I'll fix soon in 🐛 PHPUnit failure Active .
    So

    • tests actually 'succeeded' (for markup)
    • and for (the changed) JSON output we have no tests yet. I should add these after the code change is approved (either in this issue or in a followup).

    About alternative implementation

    I just want to mention this for completeness - Disclaimer: there are gaps in my knowledge

    It feels to me like, in principle, given the fact that the above 'JSON array' is apparently invalid... the Normalizer is responsible for not producing invalid output.

    So, instead of the change in the MR ("bulid/render-into-element phase"). the "render-into-output" phase should have the fix.

    That would look something like changing some code in normalizeSlots() to this pseudo code:

    if ($element->hasSlotNormalizationStyle($slot_key, CustomElement::NORMALIZE_AS_SINGLE_VALUE)
        || (ThisSlotOnlyHasASingleValue($slot_key) && MyFormatCannotHandleArrayValues()) ) {
      $slot_data = reset($slot_data);
    }
    

    Where MyFormatCannotHandleArrayValues() is either always true, or can be made to check the $format parameter for the normalize(mixed $object, ?string $format = NULL, ...) call.

    However,

    • it is harder for me to see if that has wider implications
    • if we would implement this, I'm starting to wonder if NORMALIZE_AS_SINGLE_VALUE still has an actual use in the future, or whether we should also start to deprecate that
    • I have no clue if all consumers (JS frameworks) would have problems with array values. (Hence checking the $format parameter.)
    • We are not using the $format parameter for anything yet (even though we soon will, in an upcoming change to Create a HTML-to-CE parser Active that I or Mustapha should still document).

    So, this is too 'unknown' for me to make a MR out of, unless you actually agree that this should be the direction. (The current MR that changes DefaultFieldItemListProcessor, feels more in line with... how the code is currently set up...?)

  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU
  • 🇦🇹Austria fago Vienna

    +1 on changing it like this, also on changing in the way as proposed in the MR. Still I added some remarks!

    if ($element->hasSlotNormalizationStyle($slot_key, CustomElement::NORMALIZE_AS_SINGLE_VALUE)
    || (ThisSlotOnlyHasASingleValue($slot_key) && MyFormatCannotHandleArrayValues()) ) {
    $slot_data = reset($slot_data);
    }

    This pseudo-code does not make sense to me. It'S not that Vue cannot handle that array, it's that it does not make much sense to render into a single Vue component via a slot. The body is typically rendered using a simple slot, what semantically makes sense, but that's done with something like this pseudo-code: <component content="$SLOT"></component> $SLOT must be string then and will fail it's an array. Thus, the ARRAY fails because our frontend-code excpects a string.

    That said, it seems we had this working before - although it's not so logical - and it makes frontend code more consistent to have it working again. Because of that, I turned around and fixed the frontend, so it IS working now.

    Anyway, I think we shall do this chance still, because we want to have a default output which is semantically good. And this is without the array.

    --> I re-title this a bit.

    Still, before merging - I think we need test coverage here. Ideally we could extend the unit test to cover it, but howsoever, we shall make sure we have this behavior covered by tests. thus setting needs work for that.

  • 🇦🇹Austria fago Vienna

    after commenting this, I realize that changing this won't help our default output since by default the body field *has* a summary enabled!

    Maybe we should go back and instead of doing this simply improve the default-config for our body field? Could we use the "flatten" plugin to flatten the output in two slots, like summary and body/default slot? If not, I'd be fine with simply only output the full-body field by default in the slot and not handling the summary. Folks can still change the config if the summary shall be used somehow.

Production build 0.71.5 2024