For serving the "Canvas developer use-case" better it might actually be useful to optionally load a group of JS-bundles from a configurable, absolute URLs. That way, it could become possible to define such JavaScript components without having to write custom Drupal code. That way this could be used together with any kind decoupled JS frontend where the JavaScript part could be implemented.
Found some working config:
vite: {
server: {
cors: {
origin: ['http://xb-dev.ddev.site'],
},
},
},
nitro: {
routeRules: {
'/nuxt-component-preview/entry.js': {
cors: true,
headers: {
'Access-Control-Allow-Origin': 'http://xb-dev.ddev.site',
'Access-Control-Allow-Methods': 'GET',
},
},
'/_nuxt/**': {
cors: true,
headers: {
'Access-Control-Allow-Origin': 'http://xb-dev.ddev.site',
'Access-Control-Allow-Methods': 'GET',
},
},
},
},
So it seems CORS is really the simpler variant. Let's better clearly document CORS has to be setup correctly + make it easy in our nuxt component-preview helper.
Thx! I reviewed all the changes and they are generally solid. I'd say we can move on with releasing this and create individual issues for everything that might pop up during testing. Also moving this to version 3.x, since this going to be released as 3.x.
I think the following should be in a change record, to ease upgrading to the new major:
- config-key change
- environment variable change (OHDEAR_SITE_ID -> OHDEAR_MONITOR_ID)
Maybe a note that dependencies changed also?
Thinking about this topic a 2nd time, the CORS route seems to be better. The proxy-setup is rather complicated and might confuse people much more than a few CORS-requests running in the browser.
For nuxt dev mode, I got CORS working immediately.
For nuxt dev mode the following addition to nuxt.config.ts did the trick (replace origin URL with your drupal base-url obviously):
vite: {
server: {
cors: {
origin: ['http://xb-dev.ddev.site', 'https://xb-dev.ddev.site'],
},
},
},
When running the regular nuxt server, a different configuration is going to be required.
However, a straight-forward configuration as following did not do the trick. Investigating.
nitro: {
cors: {
origin: ['http://xb-dev.ddev.site', 'https://xb-dev.ddev.site'],
methods: ['GET'],
},
},implemented the straight-forward rename + added note for being experimental in readme
disclosure: AI helped me creating this
Thank you, all good and comes with a test. Merged.
Since this is just a test addition, decided to move on. Merged.
I think we can call the prototyping done
As seen at
📌
First-class experience builder support
Active
we have already moved into the building phase. Details on current status is best found there.
following the rename
This would lead to some API-deprecations, but I think it's all not very frequently used APIs, so I guess to would not be too disruptive.
I agree that the suggestion still makes sense + would be a rather important change, so increasing to prio to Major.
Some analysis:
* DrupalForge uses devcontainers - like GithubCodeSpaces. but it's implemented without ddev - it just launches docker-compose.
* docker-compose of DrupalForge uses devpanel images
* NextDrupal adds the node service via pm2 to the main webserver container of devpanel, this is similar to what we do this within ddev atm
I noted there are phpcs and phpstan issues also. I added a schedule pipeline exeuction on daily basis so we see this earlier + tried to fix it as part of this MR
added 📌 Test enabling layout_builder after custom_elements Active
Test case added. Disclosure: AI helped me creating this.
commenting out template_preprocess_item_list() in responsive_preview_preprocess_item_list__responsive_preview() removes the exceptoin but makes the responsive-preview list appear always empty for me
gitpod is deprecated
added a test case for rendering a layout-SDC with slots using twig-based rendering, that works as expected also
so now only the more complex case of "CE-rendered elmeents in twig slots" is left, for which we have 📌 XB: Add support for rendering ce-elements in SDC slots Active . This can be tackled when all the rest came together.
So this MR introduces the helper setSdcCustomElementComponents() for deciding whether a SDC-component is rendered as twig in Drupal or via a custom-element in the decoupled frontend. So according to the plan 📌 First-class experience builder support Active this can be used by the XB-integration to enable CE-rendering for components where we have ce/nuxt-components registered. But this is not part of custom elements, but will have to become its own module or some optional XB-feature.
Given that the MR should be ready.
adding issues
A first version is ready. I think we should still extend it to cover testing slots get rendered correctly also, i.e. a twig SDC containing some other twig SDC.
Then we should test that CE-rendered elements containing twig-SDCs works as expexted.
As outlined in the parent, the more complex case of twig SDCs containing CE-elements is something for another issue later.
I totally agree "something" like extra fields, e.g. a field that returns a programmatically derived ("calculated") value, instead of a database value, makes a lot of sense in core and is definitely needed.
Sounds like you are taking about computed fields.
Attach the preview script implementation, i.e. the drupal library to load and attach. The XB-component could simply store which drupal library to attach. Custom Elements module would have to take care of deciding upon the right script to attach (e.g. Nuxt-CE-Previewer implementation).
Since custom elements wants to support multi-site/domain module use-cases, we cannot hard-code the way to preview by component. However, it should be doable to implement a generic "custom_elements/preview-component" drupal-library which gets processed accordingly by custom elements, such that modules can chime into this processing and override the library based upon request/site-context.
As determined in https://www.drupal.org/project/lupus_decoupled/issues/3520654#comment-16... 📌 First-class experience builder support Active we need a new API that allows us to preview custom elements in the Drupal admin backend, which we can then leverage in XB previews. Adding here.
fago → created an issue. See original summary → .
We've got the basic process for CE-output for XB-pages implemented in custom elements 3.x-dev now. Given that, we can start taking care about more details, opened 📌 Improve mapping of SDCs custom elements Active for that.
So from a XB perspective we need to clarify how we know when a component is mapped to CE or rendered traditionally. I think, it would make sense to default two twig rendering for SDCs and to opt-in into CE rendering by some mechanism. That could be some UI or a cli tool similar to xb-cli that scans vue components and - based upon a certain naming pattern - registers them in Drupal.
So besides registration, the most convenient approach seems to be to simply apply a naming pattern. We need our registry of place-able nuxt-components anyway, so when we have a component matching the SDC-override naming pattern we can apply it as override.
That said, it seems the next step needed is a proper XB "component source" implementation for placing components + a registry of components to place. Our components would be JS-components that render the custom-elements (e.g. in JSON representation) with some glue-JS to load the nuxt bundle. However from a CE-perspective, we don't know how the CE is rendered either.
In XB, we'd only need a quite generic component source, that can take care of the following:
- Custom element name. Property/slot information can be 1:1 the component prop/slot information.
- Provide the custom element name + property/slot information to the preview script, by rendering it suitingly
- Attach the preview script implementation, i.e. the drupal library to load and attach. Needs a way to decide upon the right script to attach (e.g. Nuxt-CE-Previewer implementation).
So, in XB this could be some sort of generic JavaScriptComponent - that holds the reference to the drupal library/js to attach for rendering.
In custom elements, we'd need a new mechanism to generate the preview JS a given element.
smustgrave → credited fago → .
I've updated the proposal in the issue summary to reflect this is released now + worked in my proposal from the previous comment.
What changes are you missing?
When we keep the parameter, there is no change necessary to ContentEntityBase + I argued why we don't to extend unit tests here. I don't see any other changes.
> And needs to be properly be deprecated with a trigger_error and CR
Not sure, this is helpful / needed in this case. Since the parameter was already doing nothing before, there is no change and no possible breakage to bug the developer about.
But if preferred, I can add it when a FALSE or NULL value is passed instead of the TRUE default.
bump. this is filling the logs. is no one else running into this?
I agree it should be deprecated for now, not just removed. Some child implementations might be using it, so silently removing the parameter would render the child implements having a weird undocumented $notify parameter. Besides ContentEntityBase I see it implemented in \Drupal\simple_oauth\Authentication\TokenAuthUser::set
I'd argue the change needs no unit tests, even when removing the parameter, there is 0 change in behavior.
I added a MR for deprecating it.
Sure! Updated it.
I created a quick fix for that. I fixed it for all the tests where I found the issue (checking various constraint validator tests).
Disclosure: I used AI to help me with this.
jjchinquist → credited fago → .
this is about this comment: https://www.drupal.org/project/drupal/issues/2142991#comment-8309595 →
The local variable $this->typedData is named very confusingly and should be $this->typedDataManager. Those couple of tests have this issue, no biggie at all, but makes sense to fix.
almost, 📌 Avoid dynamically defining constraints Postponed: needs info is the only still be left todo. That said, I don't think we need this META anymore, the remaining single todo can stay open. Let's close it.
Checking on this, it's still relevant.
The metadata (=field definitions) is missing out on constraints that are defined via \Drupal\Core\Field\FieldItemList::getConstraints(). Some modules and even core use this to define additional constraints, like \Drupal\Core\Field\EntityReferenceFieldItemList::getConstraints. While this code does not make use of the field value, it could, but that's not intended. It would still make sense to improve this API to make sure constraints are defined not dynamically, e.g. by replacing the method with a static one - like \Drupal\Core\Field\FieldItemList::processDefaultValue.
This is of course an API change, so would need BC and communicated deprecation.
While the API change would be happening on typed data level, the impact would be mostly for field type developers. So moving this to entity field api to get some feedback of entity system maintainers.
Proposal:
- Deprecate TypedDataInterface::getConstraints()
- Add "static public function getValidationConstraints(DataDefinitionInterface $definition)"
So that means all the implementations of FieldItem::getConstraints() would have to follow and implement the static getValidationConstraints instead.
So moving into the entity queue for sub-system maintainer review.
I do think people already are used to this now.
The way I'd see it a field is just a special kind of property, and with that the loadByProperties() method makes sense for fields also. However still, I think some people are confusing by loadByProperties() and do not think they can use it for fields also.
That said, I do think it still makes sense to clarify this better and e.g. just add an alias loadByFieldValues() in ContentEntityStorage. But that will be up to the new entity maintainers to decide. So setting the right tag.
I think major is ok, it's a quite important aspect still, so leaving the priority as it is.
Fixing it on entity-access layer makes sense, but it's not solving the issue I reported here. The UI already has no way to delete filter formats, it can only be disabled. This issue is about unexpected behavior when you run the code, with drush, or via an (post)-update hook/function, that does not matter:
\Drupal\filter\Entity\FilterFormat::load('restricted_html')->delete();
So when the filter format is still referenced in field config, those fields get deleted, including the content. I'd argue this is very un-intuitive behavior, since this is not visible or clear from the code executed.
Indeed, deleting the filter-format might result in data-integrity issues. But I'd argue that's an intuitive. possible consequence from deleting a filter format manually like this, but loosing not touched field-configuration including the deletion of if its data is not.
Additionally, you can already delete filter formats that are in use with code snippets like this - so that "data integrity" issue when deleting filter formats via code is already pre-existing. The described problem only applies if the format is referenced as a "allowed-filter-format" in the field. However, if no filter formats are referenced, filter-format usage is not restricted (so it might be in use) and deleting a filter-format would not delete any fields. Thus, this "data-integrity issue" is not new, but the suggested change makes the behavior consistent in both cases.
I must say, I don't see an issue with the "data-integrity" problem, as mentioned, that's intuitive to me when you delete a filter format like this. However, if we decide it is an issue that shall be tackled instead, we should also cover the case of un-referenced filter-formats which are currently not referenced in field configs at all. That sounds like a quite complicated problem to solve though.
#3537942 got merged, thus I've update the MR. Now it can be reviewed easier.
Thx merged I'm going to rebase the other MR then.
> The only remark that needs another look is from the .module file (template suggestions but the only template there currently exists is for renderless-container).
Yes, it generally allows you to add a template for special custom-element, e.g. you could add "custom-element-drupal-markup" if somehow you want to customize it. I think that makes sense and comes with no down-side compared to only adding a single suggestion.
ready. this is based upon
📌
Integrate experience builder with entity-custom-elements-generation
Active
.
disclosure: I've leveraged AI to create the MR.
ok, new MR is ready with a couple of improvements/fixes:
- adds new renderless-container for better wrapping of multiple children, as described above
- adds template-suggestion for overriding markup output by tag
- implements renderless-container in both markup and json serialization + adds test-coverage for that
- improves XB field-formatter to avoid nesting, XB-fields are single-valued
- improves XB-field-formatter test to assert the flat structure when renering multiple components
I figured the resulting structure is weirdly nested though, this needs improvement. I'll take a stab on it before merging this.
I figured we need a way to make the normalize flatten a custom-element with slot-content in a way that the custom-element turns into a list of custom element elements. Without that it would become tedious to flatten things, since do not want to have a helper like convertRenderArray() to return null|¢ustomElement|CustomElement[]. We should be able to organize a list of custom elments in a single object, that helps to keep code clean.
Thus I'm taking a stab on adding this here. I'd call it "renderless-container", similar to the concept of "renderless-components" in the frontend world. This is not very nice, but expressfull, so when you run into it it should be clear what it is. Since the element is not rendered, we cannot really render the attributes either, so the result is that it only renders the children/slots.
Merged into 3.x-dev! Next step: 📌 Integrate experience builder with entity-custom-elements-generation Active
I do think the best and right now also the easiest way to do this is by adding a new, dedicated ce-field formatter.
The frontend can take care of showing the XB-content as a full-page and deal with other fields as needed.
This goes inline with the layout-builder integration which we recently improved to have fields next to the layout, so just keeping the output as yet another field makes sense.
I'd not use the "auto" processing to support, with the formatter we are more flexible when we need settings in the future + the existing "auto" output might be an interesting alternative for some folks, it's a verbatim output of the internally stored structure, let's better keep that there.
What probably makes sense is that we take care of auto-configuring the formatter properly in the future, so the auto-configuration based upon the field-type is something we might want to look into, but that's a generic improvement, and not in scope of this task.
Oh I see, I'm sry to have caused this issue and it ate your time. The 1.x/0.x separation was new to me. :-/
finally got it green on CI also. Getting CI work was a bit tricky since experience_builder only works with Drupal >11.2 now.
Disclosure: I've used AI (Claude Code) to help coding this, in particular the test case. I worked over all non-test code and reviewed all code changes of course.
As a preparation for figuring out how to integrate best, here is an AI-based summary of how XB takes over entity generation.
I've added a MR with test-coverage which successfully renders the XB output into a tree of custom elements.
It's for sure not perfect, but it should lay the foundation on how we do it, so we can improve the output as later step.
Next, we need to integrate this into the entity rendering pipeline either via auto-processing, a dedicated formatter, or global detection in the entity ce-display.
Thanks for the quick reviews!
It's not that we specified the wrong property, it's that the inherited \Drupal\Core\Field\FieldItemBase::mainPropertyName() does not make sense for XB's field type.
yep, exactly!
>here was 1 commit, now there's 8?!
They were there from the beginning.
Maybe, there was some glitch between 0.x and 1.x when I created the PR, I was confused about the branches. But the diff is correct, so it should be fine to just squash-merge it.
This actually concerns text fields provided by text module.
Thank you. I added the test case, verified it fails first, and then put it on top of your changes, to see it pass! All good, this seems ready then!
I also ran into this and opened 📌 Deleting filter formats may result in data loss Active for issue I faced as consequence: deletion of text item fields that have the text format as "allowed_format". I do think we should fix both, this issue and 📌 Deleting filter formats may result in data loss Active .
Checking on the proposed solutions I think both are viable. I agree that MR 12404 is the best approach though. Since it's a generally new feature, I guess it's best done over at [#579743]?
With onDependencyRemoval() we could remove the dependency and let the field and data survive, what I think would be acceptable given the deletion.
I'd prefer to deny the deletion when the field has data though, but that does not seem to be properly doable. The only documented exception for ConfigEntity:delete() is EntityStorageException what seems not suiting.
The code generating the dependency on filter-formats in text-fields is that method:
/**
* {@inheritdoc}
*/
public static function calculateDependencies(FieldDefinitionInterface $field_definition) {
// Add explicitly allowed formats as config dependencies.
$format_dependencies = [];
$dependencies = parent::calculateDependencies($field_definition);
if (!is_null($field_definition->getSetting('allowed_formats'))) {
$format_dependencies = array_map(function (string $format_id) {
return 'filter.format.' . $format_id;
}, $field_definition->getSetting('allowed_formats'));
}
$config = $dependencies['config'] ?? [];
$dependencies['config'] = array_merge($config, $format_dependencies);
return $dependencies;
}
As seen, it only adds dependencies for the "allowed_formats". This seems bogus, since there is no larger harm when a filter-format this is a allowed-format is removed than when a filter-format that is not referenced is removed. When all filter-formats are allowed, no dependency is added, the filter format might be used, and it can be deleted. What happens is that the field and its data is not removed, it just gets a reference on a selected filter format.
On the other hand when a filter format is removed that is part of the "allowed-formats", since there is just one entry less that users can select. The more severe data integrity issue is the same as before.
That said, the right fix here is probably to add a config dependency on all filter-formats when the list of filter-formats is not restricted.
Then, given that we already have the issue that filter formats leads to data with invalid text-format-references when no "allowed_formats" are set, implementing onDependencyRemoval() to avoid field-data deletion in case of "allowed_formats" usage seems reasonable.
Thank you Kars-T. Let's also try get some more feedback here before we move on with the new 3.x
Trying to reach out to yahyaalhamad - who was the last active maintainer. -> Assigning the issue to him, unfortunately the contact tab is not open.
I've reached out to the module maintains who have an open contact tab (Kars-T, focus13)