#stable blocker

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.

Looking at #12 I think some of this we should ask the component to export this information

+1

I think we should split off the preloading in #12 off into a separate issue as it is more involved.

WFM, but then that needs to become a beta blocker. Made it so in #3538273-3: [PP-1] Use AST to identify resources fetched by CodeComponents so they can be preloaded/prefetched → .

Thanks, both of you, looks great! 🤩

Opened a draft MR, still to go - e2e test

Created draft change record → and started on update path now we're beta stable

Added ✨ [PP-1] Use AST to identify resources fetched by CodeComponents so they can be preloaded Postponed for part 3 of #12

After a spike on @babel/traverse, I've gone back to reviewing the original goal of this issue, which is to remove the string matching in CodeComponentDataProvider::getRequiredXbDataLibraries

But this was expanded in #12 to also allow for identifying URL requirements so we can add <link rel="preload"/>. However this expansion of scope is what is causing the difficulties in #19

I think we should split off the preloading in #12 off into a separate issue as it is more involved.

I'm going to focus just on CodeComponentDataProvider::getRequiredXbDataLibraries here

Spent some time here trying to make use of the existing dataFetches property in the slice to populate the dataDependencies property on the backend.

The issue we have is one of timing.

We clear that property every time the source code changes in collectImportedJsComponents, and then as the code calls the methods, these entries are added back.

The issue is, at the time we update the autosave, we've just cleared the property and hence it is always empty.

To try and do this in a non-async fashion so we can get a definitive value without needing to wait for the iframe to post messages I imported @babel/traverse to attempt to gather this from the ast.

Some of these are easy, like detecting getSiteData and getPageData.

Where it gets tricky is finding and resolving the value of things we call fetch with - I can use traverse to find all CallExpressions where the callee.name is fetch. We can also look for the getResource call on the JSON API client. But finding the calls and resolving the value are two different beasts.

fetch('https://example.com') is easy to resolve because we have a string literal.

But fetch(url); is a bit harder because we have an identifier instead, and then have to resolve those too. e.g. url could be the result of a function... And it just keeps getting more and more complex.

I'll see if I can find a way to do something that doesn't require clearing the dataFetches values from collectImportedJsComponents but I'm wary of retaining values from the last preview re-render as they could obviously get out of sync as the component changes.

Continuing work here

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

So to achieve #12 I think we should probably split `dataFetches` into groups - one for uris and one for utils like getPageData
But this all seems doable. Have pushed my WIP and will continue on monday.

Unassigned in case someone else wants to pick this up in the meantime.

Neat. following the existing pattern I was able to add generic fetch support, how did I not know about that tab until now, amazing

Just realised there's an addDataFetch reducer and a third tab in the code editor 🤯

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

The blocker is in, @effulgentsia asked me to look at this one.

Looking at #12 I think some of this we should ask the component to export this information

For SSR we will likely need additional exports for components to return information like prefetched data and cache metadata

Should we consider adding an additional method to allow the component to return preloads? Parsing that from the AST is going to be fraught.

Imports we can parse with the AST, but things like urls inside fetch calls is going to be brittle.

export const getPreloadUrls = () => (['/some/url', '/some/json/api']);

ADR/discussion/research on SSR is here for review

larowlan → credited bnjmnm → .

larowlan → credited wim leers → .

Coming back around to this, as per #10.

A new components config with one version looks like this:

uuid: d973b1d2-df01-4f89-a309-fe83f34c9d9a
langcode: en
status: true
dependencies:
  config:
    - experience_builder.js_component.test
active_version: 8fe3be948e0194e1
versioned_properties:
  active:
    settings:
      prop_field_definitions: {  }
    fallback_metadata:
      slot_definitions: {  }
    label: test
    id: js.test
    provider: null
    source: js
    source_local_id: test
    category: '@todo'

New versions created adds them under `versioned_properties` as expected but as you can see above the hash `8fe3be948e0194e1` is not in `versioned _properties`. So either the above isn't structured as expected (though I'm sure there are a number of tests around this) OR the original constraint that was commented to add later maybe stale?

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

Thanks!

🥳

Removing the prefix, because that's already the reality of XB anyway :)

This can be worked on now.

Failing test-only CI 👍

I had only nits. Over to @tedbow for final approval given this is firmly in the component :)

Looks great — thanks!

wim leers → credited larowlan → .

Going to pause and create an ADR around the requirements here and for SSR

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.

It seems this is kind of a duplicate of ✨ Usage info for code components with unpublished changes Postponed (because it's hard to find).

@lauriii in #8:

What would be extremely useful is having a list of usages of the component.

We already have that infrastructure and information, since 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active . We just don't have it in the modal dialog, which is what I suspect you're hinting at?

If so, that makes this even more closely related to ✨ Usage info for code components with unpublished changes Postponed !

Could we make the soft delete as an explicit feature?

Yes, but what does "explicit feature" mean? 👈 confused me even more, perhaps you meant "components"?

Left a comment about how I think we can fix this in a more generic way

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?

I was doing it for both. I hadn't finished wiring up <xb-preview> yet, but the SDC elements were working as-is (as you'd expect). I'd still like to explore this approach as it sidesteps the need to detect use client and unwrapping altogether.

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.

Ok, sounds like this is the next step for this issue.

I just got is that ultimately what we're trying to emulate is SSR. So, what if we make 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.

Yep, we could do that but we'd still have to make the decision about interactivity with use client.

So it sounds like the next logic steps here are:

1. Change the logic back to detect 'use client', but also consider dependencies AND useSWR or next-image-standalone as a trigger for preventing unwrap
2. Explore using renderToString to replace the outerHTML rather than rendering and then moving

The questions is, how much of this is moot if we have SSR? Should we be focusing on that instead? I think we can make that pluggable so that there are multiple SSR providers - e.g. symfony/process that calls node on the host, a HTTP endpoint that calls a separate container running an express web-service etc.

Ready for review.