There are very likely zero fields in existing Drupal sites that contain inline-only markup (i.e. with no
,
and other block-level elements). So any such SDC props are unlikely to be able to get populated by structured content.
I think inline HTML props should match to both TextItem
and StringItem
, just not to TextLongItem or TextWithSummaryItem.
For TextItem
matches, I think we'll need to take the ->processed
property and run it through an additional strip_tags()
that retains only inline HTML. That might mean we need to implement
📌
Allow use of same-shape-adapters ahead of general adapter support in #3464003
Active
first.
CKEditor 5 currently assumes
is supported at minimum
Yeah, we need to either get this fixed upstream or somehow hack around it.
Is the "preview LIES" part of this issue title still accurate? The way I read the comment linked in #3 is that there are situations where you can publish something while it still have invalid values. If those cases still exist, shouldn't those be separate issues? Meanwhile, if you can't publish with invalid values, then what do we mean by the preview lying?
Not every stable blocker needs to block an RC, but this one does.
Not every stable blocker needs to block an RC, but this one does.
Not every stable blocker needs to block an RC, but this one does.
Beta blockers are by definition also stable blockers, but removing the latter tag so as to make it easier to scan a queue of stable blockers that aren't also blockers to beta or RC.
Beta blockers are by definition also stable blockers, but removing the latter tag in order to make it easier to scan a queue of stable blockers that aren't also blockers to beta or RC.
Transferring beta blocker tag from the duplicate issue.
No arguments were made since #20 for keeping this issue open, so marking it Fixed.
#33 doesn't need to block a stable release unless we already know of or suspect a problem.
I think #13 answers #12's question to @lauriii.
And yes, implementing what the issue title now says needs to block the beta1 release, since it's a significant BC break of site data for sites that have already created component trees for individual nodes (articles). We'll need to include release notes that instruct people to manually copy (yay for copy/paste functionality in Canvas!) those component trees from nodes to page entities before they update their site to beta1 (or alphaN if there's an alpha tag that includes this removal prior to the beta1 tag).
wim leers → credited effulgentsia → .
wim leers → credited effulgentsia → .
penyaskito → credited effulgentsia → .
penyaskito → credited effulgentsia → .
penyaskito → credited effulgentsia → .
The sooner we do this, the sooner we get to reap the benefits, but I don't see why we should block a 1.0.0 release on it. Seems like the kind of thing that could non-disruptively go into 1.1.0 or even 1.0.1. Therefore re-tagging from "stable blocker" to "stable target".
I'm all for naming things well. This rename can happen either before or after a stable release.
Per 🌱 Milestone 1.0.0: Production Sites Postponed this is a requirement for an XB 1.0.0 stable release, so tagging it as such.
We can release XB 1.0.0 without this, so switching from "stable blocker" to "stable target".
Within the preview/canvas iframe, global assets are rendered via drupalSettings.xb.globalAssets
, which includes jsFooter. Can you clarify steps to reproduce the bug you're seeing?
In fact, since we haven't released the beta yet, we should get this into that as well.
If the hypothesis in the issue summary is accurate, this is a stable blocker, so tagging it as such.
I discussed this issue with @bnjmnm. I believe the correct status of this issue is "duplicate" because per #12.1 it was a symptom of 📌 Redux-aware form input components should be aware of direct element manipulation Active which got fixed.
However, this issue was not the one encountered by the Webform block. Instead, Webform was running into 🐛 Autocomplete machine name handling does not match to machine name Active , which was a symptom of 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active so is correctly marked as a duplicate of that one.
Unrelated to either this issue or to 🐛 Autocomplete machine name handling does not match to machine name Active , there is a remaining bug described in #3523496-40: Block component instance form values not processed by validation/submit handlers → .TC3. @bnjmnm is working on diagnosing that one and will open a new issue for it based on what he learns.
For MySQL itself, it's a bit more vague: https://launchpad.net/ubuntu/noble/+package/mysql-server but de-facto you still get 8.0 it looks like. However I wonder if they're going to switch that mid-release to 8.4 (supported until 2029 according to https://endoflife.date/mysql) as 8.0 goes EOL). So maybe it would be fine to require 8.4 according to #8 if ubuntu installs get updated anyway.
I don't see what's vague in that link. It looks clear cut that Ubuntu 24.04 is on MySQL 8.0. I don't think there's any precedent for Ubuntu to raise the minor version of MySQL (or any other significant package) within an LTS branch that has already been released. Based on all of their track record, I expect Ubuntu to security patch MySQL 8.0 for as long as 24.04 is supported (till 2029 minimum and beyond that for extended support). I think #8 would clearly dictate having Drupal 12 require MySQL 8.0, not 8.4. I don't know if we have consensus on #8, but I'm still in favor of that. Most people are not going to upgrade from Ubuntu 24.04 to 26.04 within the first year of Drupal 12 being out, and while it's true that those people can remain on Drupal 11, I don't think we gain enough from raising the MySQL minimum to 8.4 to be worth the tradeoff in holding back Ubuntu 24.04 users from upgrading to Drupal 12.
Oh! Well that seems like something that's both relatively easy to do and would reduce confusion for beta testers, so tagging it as a beta target, but not a beta blocker.
Closing this as outdated. The Undo and Redo buttons work, and if there are future designs to change their location, new issues can be opened for that then.
Untagging "stable blocker", because XB 1.0.0 does not require a public extensions API. That can come in future minor 1.x releases. The goal of XB 1.0.0 is to get it into people's hands for building and deploying real websites with it, and they don't need to extend the XB UI to do that.
Initially we were only planning to postpone Workspaces to a later 0.x version, but now we're postponing it till after 1.0.0, so re-titling this issue, and we may want to adjust the language in the MR as well.
Where Workspaces will become increasingly crucial is for editing individual nodes within XB, but that's also something we're postponing until after 1.0.0. 1.0.0 will just be for Pages and Content Templates.
I agree in principle that ADRs are valuable and that they're more valuable sooner than later, so I'll try to make time for this when I'm able to, but we're not going to block a stable release on getting this ADR published.
Removing "stable blocker" tag but we'll want to fix this as part of the Content Templates work, so setting the issue parent accordingly.
See #3503038-15: Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances → . I'm very much in favor of this issue, but we're trying to pare down the "must haves" for a stable 1.0.0 release in order to unblock a stable Drupal CMS 2.0 release as soon as possible.
I read #10, but I don't think this is a stable blocker. We'd be happy to merge this once it's ready though, so by untagging it I have no desire to slow this issue down.
For XB, the definition of stable is different than core's. We'll be adding @internal to almost everything and relaxing that in 1.x minor releases. The main goal of an XB stable release is to unblock a Drupal CMS 2.0 stable release, allowing people/agencies to build and deploy real sites with that. But XB 1.0.0 won't yet have a proper PHP API surface for contrib projects to perform more advanced integrations than what DCMS 2 exercises, except to the extent that issues like this one just happen to get done ahead of a 1.0.0 release.
Now that we're effectively in beta, changing route names requires a BC layer
I haven't reviewed the MR to have an opinion on whether or not we should change the route names. But if it seems like the new names are better, then we can do so without providing a BC layer. For XB, beta only means data stability, not API stability.
Question about the tags added in #5: what contrib project is this blocking?
Removing the "stable blocker" tag, because we have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker, and that's the only part of this that seems like a must have before a 1.0.0 release. Implementing #2 from this issue's summary or a hybrid solution allowing some blocks to be #1 and others #2 seems like it could happen after a 1.0.0 release. Please re-tag if you disagree.
Untagging "stable blocker" because I don't see why we couldn't add client-side validation at any time, including after a 1.0.0 release, but please re-tag if you disagree. We already have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker for the server-side part.
Closed 🐛 The navigation prevention inside the preview doesn't always work for code components Active as a duplicate of this. That one was tagged as a stable blocker, so transferring that tag to here.
Closing this as a duplicate of 🐛 Drupal.behaviors.xbDisableLinks doesn't disable links added to the DOM later Active .
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.
Downgrading priority and removing the "stable blocker" tag because I don't think there's too many cases left where uncaught exceptions make it through to the generic "unexpected error" message. We can re-raise priority or re-tag as a stable blocker if we start seeing more of those. Regardless of priority or whether it's stable blocking, addressing this would be good for maintainability, so leaving that tag in place.
For XB 1.0.0, we don't need to support editing individual nodes, or other entity types other than Page, within XB. That's separate from being able to make content templates, so I commented on that separation in #3518272-3: [PP-1] Validate that content templates are attached to content entity types (+ bundles) that fulfill XB's requirements → .
We can add the ability to edit individual nodes, custom blocks, paragraphs, etc. within XB after 1.0.0. Until then, those things can be edited outside of XB.
Also, what might make sense here is for XB 1.0.0 to only allow content templates for nodes, and punt any support for creating templates for other entity types (e.g., paragraphs, custom blocks, etc.) to after 1.0.0.
The assumption the way this issue is worded is that XB should only allow content templates for entity types and bundles for which XB will be able to allow editing individual entities of. For example, only entity types that implement EntityPublishedInterface
. But I wonder if that's correct, so assigning to @lauriii. Because another way we can go here is to allow content templates for all entity types and bundles, even ones for which editing the individual entities can only be done outside of XB. For example, we could release XB 1.0.0 with the ability to edit content templates for nodes but not yet the ability to edit the individual nodes within XB. And if we can do that for nodes, then why not for content entity types that XB might never provide the ability to edit the individual entities of?
griffynh → credited effulgentsia → .
It's good for maintainability for our test coverage to include the most useful scenarios, but I don't think this needs to block a stable release.
This seems like a good issue for improving code quality/maintainability, but I don't see why it needs to block a stable release.
Just now realized we already had the "maintainability" tag, so perhaps re-using that is better than starting a new "code quality" tag.
Starting a new tag for issues that would improve overall code quality, but that I don't think need to block a stable release.
I think the original scope of this issue is done. #3535038: Auto-select or publish global Asset Library when needed to publish Code Components → is a child issue that would be a nice UX improvement, but I don't think we need to keep this meta issue open for just that. This issue's summary lists things for post 1.0 scope, but I think that should be a new meta issue and by definition not stable blocking.
Setting this to RTBC first before marking it Fixed in case there's something I'm missing and someone wants to make the case for why it's valuable to keep this open.
Removing the "backport" tag since it doesn't apply to what's in the issue title and #7 already cherry picked the part that's not in the issue title.
This is one we'll want to cherry pick to 0.x after it lands in 1.x, so tagging for that.
I love the creative thinking in #33, but some questions/thoughts:
The idea was to build a single component tree with the SDC and block components alongside the JS components and then use the preact render call on that inside the top level xb-preview custom element. Its a bit like what we're doing with hyperscriptify but on the preview side.
Are you thinking we only do this when we're previewing inside of XB? Or when viewing the page on the published site outside of XB as well? If the former, I'm concerned about the potential for the preview to lose accuracy if it's following a significantly different rendering path vs. viewing the page on the published site. If the latter, I'm concerned that having the whole page (including SDCs and Blocks) running inside of a single Preact VDOM tree ala hyperscriptify isn't sufficiently battle tested (us dogfooding it within XB UI for the right sidebar does count for something, but I don't think that gives us enough confidence in it to force it on people outside of XB UI) and might not be desired by people wanting to adopt JS components gradually. The advantage of Astro is that it's a mature and widely adopted framework where a lot of the potential bugs with nesting and intermixing interactive islands and static HTML have already been worked out.
The reason this is forcing us to chose between unwrapped and not is because we're treating each component as its own island
I don't see it this way, or rather, I don't see it as a negative. The way I see it, Astro is a well-loved framework for building content-driven sites, exactly the kind of sites XB is for. XB "just" adds the bonus of a really good UI and really good integration with a really good CMS. When building in Astro, you choose which components are server rendered (by default, all of them) and which are rendered as client islands (via a client:*
prop/attribute/directive). The former get rendered without any wrapper markup. The latter get rendered with the <astro-island>
and <astro-slot>
wrappers. Given how successful and well maintained Astro is, I don't really see any of that as a problem that we need to improve upon.
The unique thing introduced by XB is that within XB, people aren't writing .astro
files by hand, so we don't have client:*
directives to tell us which component instances need to remain wrapped. But we don't need to re-invent that from scratch either: React already defines the idiomatic way to do that for React components: the 'use client';
directive.
I think the earlier MR here (before I derailed us with comment #9) was on the right track. However, I think the logic needs to be: if the component has the 'use client';
directive, or if one of its dependencies does. But that needs to include dependencies that we're already tracking (i.e., other code components) as well as import dependencies that we're not yet tracking (swr
and next-image-standalone
). So perhaps the front-end could add some logic to add 'use client';
to a code component that imports either of those two? So the backend only needs to check for 'use client';
on the code component or its code component dependencies?
the unwrapping code is doing insertBefore rather than moveBefore which means we're losing events etc. moveBefore is only supported in chromium based browsers so we can't use that.
Yes, I can see why my original code that's in the issue summary is naive, since elements get inserted to the DOM, then removed, then reinserted. What a shame that moveBefore() isn't available on FF and Safari. But a new realization I just got is that ultimately what we're trying to emulate is SSR. So, what if we make <unwrapped-astro-island>
emulate that better, by using renderToString()
(same as server.js
within @astrojs/preact
) and then replacing its .outerHTML
with that? This would result in the rendered HTML getting inserted into the DOM just once.
#31 is probably contrived in that why wouldn't the component author just move the <li>
tags from ListItem
into List
, which would then be more robust to the item slots getting populated with anything, but there are probably real use cases out there that are less contrived, so #31 is just intended as illustrative.
A specific (though contrived) example... it should be possible to make a List
Code Component that does:
return(
<ul>
{item1}
{item2}
</ul>
)
And for item1
and item2
to be slots that get populated with ListItem
components that do:
return(
<li>FOO</li>
)
And so long as these are non-interactive components, for the DOM result to be:
<ul>
<li>FOO</li>
<li>FOO</li>
</ul>
Instead of:
<ul>
<astro-slot><astro-island><li>FOO</li></astro-island></astro-slot>
<astro-slot><astro-island><li>FOO</li></astro-island></astro-slot>
</ul>
Sorry, didn't mean to change the issue attributes.
I'd love to see a proof of concept of #26. I'm curious how you manage to get around the need for some kind of wrapper element for each component. React's createRoot()
and createPortal()
both require a DOM element container that they can completely take over. Does your idea somehow work without any such wrapper element? If so, even if it's bound to Preact and Astro internals, can you elaborate on the approach? Maybe we can find a way to implement a similar approach with the other Astro-supported JS frameworks as well?
I think the problem with #28 is that display: contents
doesn't affect CSS nth-child selectors and similar. So the presence of <astro-island>
elements isn't completely transparent. It's basically the <div>
soup problem, just with <astro-island>
instead of <div>
. Ideally, XB wouldn't insert any extraneous markup at all into the DOM, so that component authors are in 100% control of their markup.
I think it's okay for client-interactive components to have <astro-island>
wrappers: Astro is a popular framework, so this is easy to explain and point people to docs and blog posts about it. But for non-interactive components, Astro would normally SSR (or SSG) those, and ideally so will we once we add optional SSR support to XB, so the goal of this issue is to output a DOM that's the same as what we would end up having with SSR.
I haven't reviewed the whole MR, since others have, but the inputBehaviors.tsx
change looks correct, so I approved it since that was the only file not covered by other code owner approvals.
I would definitely not have expected useSWR() to require using 'use client'.
Same. But according to https://swr.vercel.app/docs/with-nextjs, useSWR()
is intended as a client API and is not supported by server components. And when I google search "does useSWR require 'use client' directive", Google's AI response is "Yes, useSWR requires the 'use client' directive in your component file". So us requiring it wouldn't break existing React ecosystem expectations with respect to this.
Given how close this MR is, tagging it as a beta target, because doing it after beta1 does involve a small BC break of site CSS, even if rare, per #3.
Other than #9, I think this MR looks great.
"Needs work" for the feedback above, but also a more philosophical question related to it:
If someone uses useState()
, then they need to explicitly add 'use client';
. That makes sense and I think is a standard requirement for any React codebase that uses RSC. However, are we okay that component authors will also need to add 'use client';
if their component calls useSWR()
even if it has no other client-side code? In principle we could eventually add support for useSWR()
to be SSR'able. But until we do, is it reasonable for component authors to have to know that our current implementation isn't and that's why they need to add 'use client';
?
I considered whether we should make #unwrap
check for either 'use client';
or useSWR()
, but the downside with that would be that if in the future we do make useSWR()
SSR'able and then change #unwrap
back to just be based on 'use client';
only, then that would be a BC break for existing components, since components that we were previously keeping wrapped with <astro-island>
would start getting unwrapped, so existing site CSS could break for these. Therefore, I think keeping #unwrap
tied to only 'use client';
is preferable in that component authors will be in control of if/when to remove their own 'use client';
directives on a component by component basis, once we add the ability for useSWR()
to be compatible with SSR'd components.
I guess my questions are:
- Does my reasoning above make sense?
- Even if it does, are we concerned about it being a WTF for component authors in the meantime needing to add
'use client';
for components that aren't interactive but need to fetch data?
I think everything I had in mind when I wrote #11 landed already, so setting this to RTBC, but please re-postpone it if there are other tedious-to-rebase MRs we're still wanting to get in before this one.
penyaskito → credited effulgentsia → .
We're not targeting this for beta anymore. I'd still like to resolve it relatively soon after beta1, but the "stable blocker" tag plus Critical priority is enough to keep this on our radar for that.
We're not targeting this for beta anymore. I'd still like to resolve it relatively soon after beta1, but the "stable blocker" tag plus Major priority is enough to keep this on our radar for that.
"Needs work" for the question about refetch
above.