Code Components should render with their auto-saved state(if any) when rendered in the XB UI

I bumped it to major only because I think we should decide sooner rather than later if this going to need changes to the system outside of Code Components. I made possible way we could do this that might just apply to the Code Components code

"Possible solution #1" seems good to me.

That solution might stop working once 📌 [later phase] ApiPreviewController must use the previewed route's controller, and override canonical content entity routes' received entity object Postponed happens. But we'll cross that bridge when we arrive there.

More short-term concern: no ComponentSource plugin should be aware of the global/request context, which this would violate. But as long as it's documented with an explicit caveat and bound to change prior to 1.0, then I'm fine with it as an interim step.

Long-term, I think this ought to be implemented as \Drupal\Core\Config\ConfigFactoryOverrideInterface. That's the mechanism that Drupal core provides for context-dependent overrides of stored configuration. Would that even be more work to implement? Because that'd set us up better for the future AFAICT. 😇

Long-term, I think this ought to be implemented as \Drupal\Core\Config\ConfigFactoryOverrideInterface.

Are we sure long term we will need this at all if rely on wse_config?

We have requirement out auto-save js components but this seem like more a UX requirement than a specification for how this would handled as far as backend storage. Could we just do real config saves, to back up work without user action and just let wse_config be responsible for making sure that doesn't pollute the live site? (relating 🌱 Identify roadblocks to staging config in Workspaces (e.g., via wse_config) Active to this issue for that reason)

I think in 🌱 Research: Possible backend implementations of auto-save, drafts, and publishing Active the 2 main reasons we decided we needed some auto-save mechanism is that not a real entity save was

1. Bypassing Validation: saving work in progress that is not valid
2. Avoid the saving overhead so we could save frequently

For 1 we could still use a simple auto-save if the validation does not pass but a real entity save if it does. In the case of saving content entities we are not using the auto-save state, which might not be valid, to then render the content entity outside of the XB UI itself or to render that entity in the XB UI for another entity besides itself. Would that be the case for the JS components? What would even happen if we tried to a render JS component config entity overridden by ConfigFactoryOverrideInterface in XB UI for another entity, say Page content entity, if the auto-save state is not valid?

For 2 do we have the same overhead concerns for JS Component config entities as we do for Content entities. If XB is going to be the default way some sites add new and edit existing Content and as well as JS Component, it still seems likely that sites will not be adding as many JS components as they would content entities.

Are we sure long term we will need this at all if rely on wse_config?

You're right: in principle not, because we'd just rely on wse_config's ConfigFactoryOverrideInterface. But there's still much uncertainty AFAICT on all things "config in workspaces", so consider it just for the worst-case scenario, where wse_config does not get us as far as we hope. (@traviscarden is working this quarter on writing the test coverage to help raise the confidence level on that front!)

We have requirement out auto-save js components but this seem like more a UX requirement than a specification for how this would handled as far as backend storage

Is this referring to the description in the issue summary?

Everything else you wrote is actually closely related to what I just surfaced in our call, and which I captured at #3499931-6: [PP-1] HTTP API for code component config entities → . I'll link to your excellent #6 from over there 👍

I agree that the scope of this issue should not be to solve it for the long term; the long term is workspaces.

For the short term, I think either the ComponentSource plugin inspecting the request and loading from the autosave record instead of the config, or implementing a ConfigFactoryOverrideInterface service that inspects the request and returns overridden config so that the ComponentSource plugin can stay in its lane, is a great option. The latter is probably better in terms of purity, so if it's the same amount of work to do either, might as well do the nicer thing even if it's short lived. "Short lived" could still be a few months, depending on how long it takes us to do all the work needed to integrate Workspaces into XB. If the config override is a lot of extra work or introduces various other headaches, then it's probably not worth it, and adding a bit of ugly code into the ComponentSource plugin isn't so bad.

Asked by Wim, I'm attaching the diagram we've been using to discuss this problem space.

⚠️ This assumes that the auto-saved code component is working. If it doesn't, then it'd be the client-side equivalent of 📌 Improve server-side error handling Active , which needs its own issue. Raised this at #3499919-20: [Meta] Plan for in-browser code components → .

#6: FYI, the test coverage for wse_config has started over at 📌 Add basic test coverage for `wse_config` with simple config Active .

#7: AFAICT wse_config does not actually use ConfigFactoryOverrideInterface: no config.factory.override-tagged service in https://git.drupalcode.org/project/wse/-/blob/2.0.x/modules/wse_config/w..., nor in https://git.drupalcode.org/project/wse/-/blob/2.0.x/modules/wse_config/s.... Looks like it was never discussed either: https://www.drupal.org/project/issues/wse?text=ConfigFactoryOverrideInte... → and https://www.drupal.org/project/issues/wse?text=config.factory.override&s... → both find zero results.
That necessarily means it must do something much more heavy-handed instead.

Given my work almost a decade ago on PirateDayCacheabilityMetadataConfigOverride for #2524082: Config overrides should provide cacheability metadata → , I'm 75% confident that what I added as option 2 to the issue summary will work fine. AFAICT we don't need \Drupal\Core\Config\ConfigFactoryOverrideBase.

Sounds like option 2 is the preferred approach, I'll start on that

Implemented option 2

The tests are failing because the experience_builder__autosave cache tag should be expected now in other tests

Tests are passing now

I added a test to address @tedbow's comment - but it appears it hasn't gone far enough

I think this is ready, with two exceptions:

• should use the route.name cache context: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...
• ::getProps() should guarantee to return an array: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...

AFAICT @tedbow's test concerns actually are already addressed: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6.... @tedbow, can you double-check? 🙏

The sort of mind-bending aspect of config overrides and cacheability bubbling, that actually proves @tedbow's concerns are tested: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...

global-regions.cy.js e2e test failed but this appears random and it passes locally for me

👹 is in 🎉

Thought about this some more and we also need a way to have the Astro island load the draft JS and CSS

NW for that

Thanks for all the work overnight

#24: D'oh! YES! 🙈

Rough plan to address #24

* Emit an event from auto save manager
* Subscribe to event in \Drupal\experience_builder\EventSubscriber\AssetGenerator
* Either new method in \Drupal\experience_builder\AssetManager for autosave _ OR _ have the component return a different asset path if in preview in \Drupal\experience_builder\Entity\XbAssetInterface::getCssPath and \Drupal\experience_builder\Entity\XbAssetInterface::getJsPath

I don't think we want the autosave (aka preview aka draft) JS and CSS files written to disk. It would mean a bunch of short-lived files that we'd need to purge at some point. What if we define a route for those to serve them dynamically? We'd need the filename to contain the ID of the component in that case. For those, I'd think we wouldn't need the hash, since we'd want the preview to just show whatever is latest in autosave storage even if that got updated between the time that the preview HTML is sent and when the browser makes the request to the asset.

It would be great if the preview asset URL was some/path/JS_COMPONENT_MACHINE_NAME. That would make a future issue with import maps nicer. For example, we'd be able to map @/components/ to some/path/ and code in one component that does import Button from '@/components/button' would work without needing every component to be listed separately in the import map. We don't need to add anything import map related to the scope here, I'm just giving context for why some/path/JS_COMPONENT_MACHINE_NAME is preferable to some/path/HASH.js, at least for the preview/autosave case. The current behavior of HASH.js written to disk is still good for the live site (real config save) case.

Works for me 👍

MR for using draft (autosave) version of the CSS/JS is up, implementing #27
Needed a fair bit of tinkering to get the astro island to support the #preview key, but that was why we added it originally.

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

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.

for #34 👍

In addition to that reason for changing the status: While working on ✨ Code Components as Block Overrides, step 1 Active , another oversight in addition to #24 became clear: \Drupal\experience_builder\Config\XbConfigOverrides should not only only apply to that route, but also only to users who have sufficient permissions: if they lack the administer code components permission, they should not be able to see draft (auto-saved) states of edited code components.
I think that can become a third MR here.

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.

@f.mazeikis thought this MR (694) was ready to be merged, but I spotted several (small!) bugs: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...

Pushed an (untested!) commit for the trickiest bit I'm contesting, but leaving the rest for @f.mazeikis to address.

I think #35 can become a separate MR, that can land separately, after !694 is merged.

\Drupal\experience_builder\Config\XbConfigOverrides should not only only apply to that route, but also only to users who have sufficient permissions: if they lack the administer code components permission, they should not be able to see draft (auto-saved) states of edited code components.

I don't think that's correct. If they have permission to use the code component within XB's page builder (if we don't yet have granular permissions for this, this is currently the same as if they have permission to use XB's page builder at all), then they should see what their component instance looks like in their content, using the draft state of the JS and CSS.

I don't think that's correct. If they have permission to use the code component within XB's page builder (if we don't yet have granular permissions for this, this is currently the same as if they have permission to use XB's page builder at all), then they should see what their component instance looks like in their content, using the draft state of the JS and CSS.

I'm referring not a Content Creator, but to an anonymous end user. We can handle that in 📌 Add access control for "code components" and "asset libraries", special case: instantiated code components must be accessible to *all* Active 👍

The last open item here is how we communicate that a config entity is in preview state.
I originally had a flag on the config entity.
Wim pushed back on that and changed it to use cache max age of 0.
I don't agree with that change.
But I also see Wim's point about not putting it on the config entity because that communicates it is something you can set, when in fact its only a run-time flag.
I think we should look to how layout builder handles this in core - https://git.drupalcode.org/project/experience_builder/-/merge_requests/6...

tl;dr I think we should make buildRenderable have an explicit 'in preview' argument, and then use that to set a flag on the source plugin.
Then the source plugin can call take care of passing #preview TRUE along to the AstroIsland element.

This means we can also do this for the Block component source plugin and therefore respect the existing 'in preview' flag on block plugins and in the future layout plugins. I worked on the core issue to add this support for blocks and make use of it in client projects - it even filters through to block plugin templates.

I think that's a win-win.

I'll hold off implementing that until I get a +1 to that approach from Wim - assigning to him for a plus or minus one.

#40: 🏓 at https://git.drupalcode.org/project/experience_builder/-/merge_requests/6... — basically: +1! 😄

Also: massive blast from the past! 😄

In relation to your interpretation

all entity loading remains unchanged compared to the state of 0.x prior to this issue — both content and config entities. This means removing XB's ConfigFactoryOverrideInterface implementation

If we remove that, then the source plugin will be responsible for loading the config entity from storage _or_ from auto save.
But we'd also need to change how AstroIsland worked (it too loads the entity). One approach could be instead of passing an ID for component ID, we'd have to pass the entity.
But I think this will get us into issues with render caching (we don't want the entity to be serialized in a render array).

So maybe that highlights a further issue with the AstroIsland element. Perhaps it is too tightly bound to the JavascriptComponent config entity. Perhaps it doesn't need to load the component. Looking at the process method, it uses the component to

a) get the component url
b) get the component label
c) attach the component library

All of these could be done from JsComponent::buildRenderable instead. This makes AstroIsland less bound to JavascriptComponent and means it could possibly be used by another source plugin.

And it means we can ditch the config override.

I'll push ahead on that basis.

Implemented #43 and it turned out pretty nice I think.

Back to NR

After a whole unexpected saga in https://git.drupalcode.org/project/experience_builder/-/merge_requests/6..., this is finally done!

Regression: 🐛 Regression after #3500386: import map scope mismatch when auto-saved code component's JS sends a 307 Active — relates to #22.

Also, another thing we forgot: we made sure to generate a experience_builder/asset_library.<name>.draft asset library … but we never actually load that 😅 IOW: we forgot to update experience_builder_page_attachments(). Created 🐛 Global AssetLibrary should render with its auto-saved state (if any) when rendered in the XB UI Active for that.

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