🇬🇧United Kingdom @f.mazeikis

During todays XB team scrum there was agreement that we should try to split the part of this issue MR that can be merged. Changes related to permissions and audit can be merged separately from the rest of the Fallback Source Plugin mechanism related work.
I've tried to do that in 3519168-subset-mr.

Would it be possible to show the values in disabled fields?

Without the Component Source Plugin, we lose the metadata about the props/block settings. We would need to rely on data stored in XB field on a node and do best our attempt go generate input fields matching value, expression and source type.
This is certainly possible, but it feels like somewhat a separate deep dive that warrants it's own issue.

With that being said, I've tried testing Wim's described scenario of data loss:

1. Enable xb_test_sdc module
2. Place couple of components on a node - Heading and XB test SDC with props and slots in laytout
3. Edit heading of XB test SDC with props and slots
4. Publish the changes
5. field_xb_demo_inpus store the following:
   {"6c280bf1-cc58-4c34-af73-7ea36d6dc1eb": {"heading": {"value": "There goes my test", "expression": "ℹ︎string␟value", "sourceType": "static:field_item:string"}}, "8cb6fa57-08df-4508-b439-cc88d9301999": {"text": {"value": "A heading element", "expression": "ℹ︎string␟value", "sourceType": "static:field_item:string"}, "style": {"value": "primary", "expression": "ℹ︎list_string␟value", "sourceType": "static:field_item:list_string", "sourceTypeSettings": {"storage": {"allowed_values": [{"label": "primary", "value": "primary"}, {"label": "secondary", "value": "secondary"}]}}}, "element": {"value": "h1", "expression": "ℹ︎list_string␟value", "sourceType": "static:field_item:list_string", "sourceTypeSettings": {"storage": {"allowed_values": [{"label": "div", "value": "div"}, {"label": "h1", "value": "h1"}, {"label": "h2", "value": "h2"}, {"label": "h3", "value": "h3"}, {"label": "h4", "value": "h4"}, {"label": "h5", "value": "h5"}, {"label": "h6", "value": "h6"}]}}}}}
6. Disable xb_test_sdc module and clear cache
7. Edit the node in question - update Heading component
8. Publish the changes
9. field_xb_demo_inpus value is updated to the following:
   {"8cb6fa57-08df-4508-b439-cc88d9301999": {"text": {"value": "A heading test", "expression": "ℹ︎string␟value", "sourceType": "static:field_item:string"}, "style": {"value": "primary", "expression": "ℹ︎list_string␟value", "sourceType": "static:field_item:list_string", "sourceTypeSettings": {"storage": {"allowed_values": [{"label": "primary", "value": "primary"}, {"label": "secondary", "value": "secondary"}]}}}, "element": {"value": "h1", "expression": "ℹ︎list_string␟value", "sourceType": "static:field_item:list_string", "sourceTypeSettings": {"storage": {"allowed_values": [{"label": "div", "value": "div"}, {"label": "h1", "value": "h1"}, {"label": "h2", "value": "h2"}, {"label": "h3", "value": "h3"}, {"label": "h4", "value": "h4"}, {"label": "h5", "value": "h5"}, {"label": "h6", "value": "h6"}]}}}}}
   Notice how "6c280bf1-cc58-4c34-af73-7ea36d6dc1eb": {"heading": {"value": "There goes my test", "expression": "ℹ︎string␟value", "sourceType": "static:field_item:string"}} is removed from the beginning of JSON blob.
10. Enable xb_test_sdc module
11. Attempt opening XB editor for node in testing
12. Observe the following exception when trying to view the node:
    Drupal\experience_builder\MissingComponentInputsException: No props sources stored for 6c280bf1-cc58-4c34-af73-7ea36d6dc1eb. Caused by either incorrect logic or `inputs` being out of sync with `tree`. in Drupal\experience_builder\Plugin\DataType\ComponentInputs->getValues() (line 174 of /Users/felix.mazeikis/work/pbx-poc/web/modules/contrib/experience_builder/src/Plugin/DataType/ComponentInputs.php).
13. Attempting to open same node in XB Editor results in a crash that prevents XB UI from fully loading, storing same kind of exception in log reports:
    Drupal\experience_builder\MissingComponentInputsException: No props sources stored for 6c280bf1-cc58-4c34-af73-7ea36d6dc1eb. Caused by either incorrect logic or `inputs` being out of sync with `tree`. in Drupal\experience_builder\Plugin\DataType\ComponentInputs->getValues() (line 174 of /Users/felix.mazeikis/work/pbx-poc/web/modules/contrib/experience_builder/src/Plugin/DataType/ComponentInputs.php).

I was about to write a suggestion that we should move "data loss and recovery" into a separate issue, but after managing to brick my XB/Drupal so easily I am no longer sure it's a good idea. Although one could argue that this is not the fault of this MR, but rather that Drupal\experience_builder\Plugin\DataType\ComponentInputs should handle such scenario gracefully.

Didn't get very far. I've pushed what I was thinking off, but it's not a complete solution yet. Preview rendering breaks, something to do with HidratedTrees and mismatch between saved inputs and Falllback, it seems. Block and JsComponent source plugins also miss logic in their respective ::valid() methods. We would need tests for this as well.

I was thinking that @lauriii testing revealed, that sometimes Source Plugins can become invalid without triggering any of our Logic, so maybe we should try to catch that at the time of instantiating/accessing Source Plugins and use Fallback source plugin in such situations.

Not even sure if this is of any use - feel free to undo my commit entirely if you think this is wrong approach, @larowlan.

From description of this I think we need "on the fly" swap Component Source with Fallback if we detect that Component Source is no longer `valid`. I'll take a stab at it.

@lauriii If I understand correctly, you manually (via Drush?) attempted to delete a Component entity and hit the error? If so, this is not what this MR is addressing. This MR addresses situations where due to module uninstall, code change or JS Component removal we encounter Component with missing/invalid Component Sources.
While the problem you're describing is valid and we should introduce some sort of safeguard against deleting Components that are in-use (made possible by #3457504: 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active ), this issue does not address this.
Testing this MR would involve uninstalling modules providing Block Plugins/SDCs, deleting Code component entities (js_component) that are in-use on Nodes by Component entities. Deleting Component entities directly is outside of scope here.

Re-tested it and this feels much closer. Components are deleted based on usage, there's even warning during module uninstall via UI - awesome.

During testing this locally I've still encountered the problem when removing module that provided Component with slots and this Component was used on a Node with slot populated by other Component.
This leads causes XB UI crash with error message "An unexpected error has occurred while rendering preview." prompting to "Try again", which re-triggers same error.

I guess this is a FE side thing and we probably can spin that into separate follow up issue.

@lauriii

In situation when a Component is used in a Layout on a Node and then the JS Component, Block Plugin or SDC is removed from the Drupal, what is the expected behaviour for such "removed" Components?
Currently this MR assigns "Fallback" plugin as source for such components, allowing us to have graceful error handling when rendering such pages and interacting with tree structures. We should be able to "recover" data completely, if JS Component, Plugin or SDC is re-introduced into Drupal.
There are some questions about UI/UX side of things, specifically:

1. Is it correct to assume that it is removed from the library listing of available Components?
2. Is it correct to assume that it should be rendered as empty div with a placeholder UUID and identifiers for preview purposes?
3. Should such component has slots with other Component instances inside of it - should those components still be rendered in preview and in live versions of the page?
4. Should "removed" Component still be visible in the Layers section of the sidebar?
5. If so, is it ok to use generic "Component" label for all such components?
6. If "removed" Components are visible in the Layers section of the sidebar, should it be possible for users to interact with them? What kind of interactions user is allowed to do? (Moving them around in Layers? Editing contents of Slots? Deleting them from Layout?)
7. If such components are visible - do we need designs for icons/messages/labels that inform users of "removed"/"fallback" status of such Components?

@larowlan Yes, there's not just an overlap - this issue is pretty much blocked on #3519168 🐛 Cannot delete JS components due to component depending on them Active .

Tested in my local, a few things stood out:

1. Upon deletion, fallback component label reverts to generic `Component` in the sidebar. I wonder if we should add something explicit, to let user know it's a "fallback"/"broken" component?
   
   
2. Disabling sdc test module resulted in XB retaining ALL of the Component entities associated with SDC's provided by the module, regardless of whether they were in use anywhere. Not sure if this is correct behaviour, as this would lead to polluting DB with unused components. I would expect only components that were used to create layouts in some way to be retained.
   

Well, I've used the signature you've suggested as a suggestion and made some changes. I also renamed the method to testRenderComponentFailure().

I have added SDC and Block Plugin that fails by default by intentionally throwing exceptions when passed TRUE as value for "crash" prop/input. By default, the value is TRUE. These new component sources are currently part of xb_test_block and xb_test_sdc test modules.

At the moment this causes failures of Functional tests for a bunch of endpoints and the recently introduced testRenderComponentLive() in SDC and Block tests. By the way, what does "Live" part of testRenderComponentLive() refers to? Don't think I've seen this term used before in XB Components context.

I am not sure if that was what @wim-leers expected? I could move them into some sort of "xb_test_failures" submodules and somewhat "limit the damage". Or is the intent is to go the other way around and make even more breakage, so we can spot all the potential issues?

balintbrews → credited f.mazeikis → .

Addressed feedback.

f.mazeikis → changed the visibility of the branch 3502371-make-page-title to active.

f.mazeikis → changed the visibility of the branch 3502371-make-page-title to hidden.

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

This is still an issue. Drupal stores the following error message:
OutOfRangeException: No data provided to evaluate expression ℹ︎␜entity:media:image␝field_media_image␞␟alt in Drupal\experience_builder\PropExpressions\StructuredData\Evaluator::doEvaluate() (line 44 of /Users/felix.mazeikis/work/pbx/web/modules/contrib/experience_builder/src/PropExpressions/StructuredData/Evaluator.php).

wim leers → credited f.mazeikis → .

This works as intended and as per error message:

page_title_block block plugin does not support previews.

So it's not a bug.
We do have issue #3502371 🐛 Handle adding page title to content Active that will change this, even if the implementation is still going to be somewhat limited.

I actually can't reproduce the issue anymore. I wonder if other work in 0.x resolved this. Would you be able to re-test this, @lauriii → ?.

I was able to recreate this issue with simple Image Component that we include in Experience Builder.
Attaching video recording of my browser.

The problem seems to have something to do with inconsistency in XB API responses and FE HTTP call payloads.
When image is selected via media library browser, the selected image is represented by media entity ID. React application makes XB HTTP PATCH calls to `/xb/api/layout/node/{id}` and potentially to `/xb/api/form/component-instance/node/11` in order to refresh preview and sidebar form. For example, this is how `model` part of the HTTP PATCH payload to `/xb/api/layout/node/11` looks like on my local after selecting image via media library:

{
  "source": {
    "image": {
      "expression": "ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}",
      "sourceType": "static:field_item:entity_reference",
      "sourceTypeSettings": {
        "storage": {
          "target_type": "media"
        },
        "instance": {
          "handler": "default:media",
          "handler_settings": {
            "target_bundles": {
              "image": "image"
            }
          }
        }
      }
    }
  },
  "resolved": {
    "image": "2"
  }
}

Note here "resolved": { "image": "2"}.

When I hit "Review & Publish" button, multiple HTTP calls are made by the FE part of the application. There's successful POST call to `/xb/api/autosaves/publish` and successful GET call to `/xb/api/layout/node/11`.
However, the successful GET response from XB API contains resolved image as an object, instead of media entity id:

"model": {
        "354a208b-33d2-4d07-80f7-5072e5fd066d": {
            "resolved": {
                "image": {
                    "src": "\/sites\/default\/files\/2025-03\/Screenshot%202025-03-03%20at%2009.21.56_0.png",
                    "alt": "kitch",
                    "width": 1840,
                    "height": 996
                }
            },
            "source": {
                "image": {
                    "sourceType": "static:field_item:entity_reference",
                    "expression": "\u2139\ufe0eentity_reference\u241f{src\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fentity\u241c\u241centity:file\u241duri\u241e\u241furl,alt\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241falt,width\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fwidth,height\u219dentity\u241c\u241centity:media:image\u241dfield_media_image\u241e\u241fheight}",
                    "sourceTypeSettings": {
                        "storage": {
                            "target_type": "media"
                        },
                        "instance": {
                            "handler": "default:media",
                            "handler_settings": {
                                "target_bundles": {
                                    "image": "image"
                                }
                            }
                        }
                    }
                }
            }
        }
    },

After this response, I get at least a single HTTP PATCH call to `/xb/api/layout/node/11` resulting in HTTP 500 error and potentially second HTTP PATCH call resulting in HTTP 500 to `/xb/api/form/component-instance/node/11`, depending on whether I have selected the component in the preview or not. Both API calls have the model part of their payloads for image looking like so:

{
  "resolved": {
    "image": {
      "src": "/sites/default/files/2025-03/Screenshot%202025-03-03%20at%2009.21.56_0.png",
      "alt": "kitch",
      "width": 1840,
      "height": 996
    }
  },
  "source": {
    "image": {
      "expression": "ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}",
      "sourceType": "static:field_item:entity_reference",
      "sourceTypeSettings": {
        "storage": {
          "target_type": "media"
        },
        "instance": {
          "handler": "default:media",
          "handler_settings": {
            "target_bundles": {
              "image": "image"
            }
          }
        }
      }
    }
  }
}

Note "resolved": {"image" part is no longer just media entity ID, but object containing `src`, `alt`, `width` and `height`. If I understand this correctly, that's what causes failed preview (losing the image part) and HTTP 500 responses.

The actual publishing of the Node still seems to happen as intended and publishing transaction stores the correct media entity ID in Node XB field inputs.
I had separate calls with @jessebaker and @longwave + @bnjmnm and me are all unsure what the data flow should actually be - IDs or fully resolved images and what is actually supposed to be happening here. We do think however that reporting this as a bug is probably incorrect - the ability to select media entity for image, saving and publishing the node with such component is not a finished and tested functionality. Can't be a bug if we haven't finished building it :sweat_smile:

We also suspect that there are multiple Media Library adjacent issues that are all related - @longwave is already looking into issue to do with default values and thinks it's stemming from the same "unfinished feature" problem. We suspect that a meta issue is required to round up the issues around Media Library and images and outlining what exactly is missing and focusing on that.

f.mazeikis → created an issue.

Updated Cypress tests, no PHP tested were affected.

f.mazeikis → made their first commit to this issue’s fork.

Made some changes in efforts to address https://www.drupal.org/project/experience_builder/issues/3504819#comment-15992301

Attaching some screenshots validating the following:

• disabling block component works
• manually disabled block components show up in the reasons list
• multiple incompatibility reasons for SDC components are listed
• enabling block component works

Addressed feedback.

Merged in 0.x, resolved conflicts, implemented Wim's suggestion.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → changed the visibility of the branch 3473275-support-disabled-block to active.

Implemented the last suggestion, addressed failing CI on ESlint and merged.

Addressed nits, updated one failing test, waiting on CI.

f.mazeikis → made their first commit to this issue’s fork.

Actually, found an issue, posted comment on MR.
TL;DR: Until Code Component is not saved with _something_ in it's CSS field, preview doesn't include draft CSS changes.
Attaching screen recording for clarity.

Resolved merge conflict, tests passing and the code makes sense. However, trying this out on local I see no draft CSS changes being applied to the actual preview. This seems to be regression, as few days ago at least saved CSS of the component would show in preview, but now neither saved nor draft CSS is being applied during preview. Looking into this.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → made their first commit to this issue’s fork.

f.mazeikis → changed the visibility of the branch 3473275-support-disabled-block to hidden.

wim leers → credited f.mazeikis → .

Addressed the feedback, there's still a couple of questions in MR, but it's ready for another review pass.

f.mazeikis → made their first commit to this issue’s fork.

Merged in 0.x - a lot of changes since. Will try to spend some time working on this issue.

f.mazeikis → created an issue.

wim leers → credited f.mazeikis → .

Reviewed and tested - great work!

I’ve added ednpoints to openapi.yml, but I’ve intentionally stopped myself from adding all the methods and examples, as I am not even sure if that’s reasonable thing to do, if FE doesn’t use it yet. Happy to expand this, if that will be the in the review feedback.
The other thing is !505 was merged in and resulted in conflict that forced me to make some changes to ApiConfigControllers and XbConfigEntityHttpApiTest , so changes in MR are slightly beyond just addressing the previous review feedback.

wim leers → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

wim leers → credited f.mazeikis → .

Reviewed and approved. It seems MR still needs approval from @tedbow or @effulgentsia.

tedbow → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

balintbrews → credited f.mazeikis → .

f.mazeikis → made their first commit to this issue’s fork.

Looks good, approved

Re-ran pipeline and it's now passing. Code looks good too - certainly an improvement, even if we still need to come back to this once we have solutions for datetime and image (and entity references in general) props handling.

I've created #3475584 📌 Add support for Blocks as Components Active to start working on "Blocks as Components" requirement. It's based on @larowan work started in MR68 (closed). It's limited scope, focusing only on blocks for now and using the "Component Source Plugin" approach, as per #12 🌱 [META] Support component types other than SDC Needs work .

I wasn't marking it as blocking as it looked as if the issue would be merged anyway and we could have started this issue while the work continued on #3469609 📌 Prepare for multiple component types: ComponentTreeStructure should contain Component config entity IDs, not SDC IDs Fixed . And it is merged now.
I've moved most of the code from MR68 (closed) by @larowlan and reached out to him on slack as I couldn't retain his commit signature.
This is not a working commit and will be used as starting point.

f.mazeikis → created an issue.

All the feedback from me and Dave has been addressed, moving to RTBC

Additional docs, ADRs and context is really neat!
Posted some nitpicks and minor feedback about sections that I personally found difficult to follow.

Your self-review questions on MR and contextualisation of commits was really helpful.
There was a lot of context and information to go through, so it took some time.
But at the end I feel like I know much better what is happening in that MR and in general with Props/Shapes/JsonInterpreter related part of backend.
Hence the lack of nitpicks - it is a significant improvement to clarity when it comes to namespacing, docs and inline docs.

Well, the MR is mostly clarifying, removing dead code and updating docs. I don't have any nitpicks 😅

My only comment is that perhaps we should file an issue for #note_374141, just so we don't lose track of that it still needs to be addressed.

wim leers → credited f.mazeikis → .

Applied suggestions, converted controller into a service, added comments.

f.mazeikis → created an issue.

f.mazeikis → created an issue.

Addressed additional feedback, waiting on more feedback on 2 unresolved threads in MR.

Bonus idea: a "Refresh" button that calls ::clearCachedDefinitions() + ::getDefinitions(), to force a refresh :)

This already happens on every page load for the "Disabled components" page. Would you prefer that not to happen and introduce a separate "refresh" button to trigger this behaviour manually?
Other than that, I've addressed (or at least replied to) the feedback on the MR.

I think I've addressed all of the feedback.

Pushed the changes so far, don't want to polish this too much before we agree it's going in the right direction.