XBEndpointRenderer & processResponseAssets() do not support `ajaxPageState` ⇒ duplicate CSS/JS loading

Hi kristen,

Looking at the Network logs, when you drag a new "Icon Card" and you upload a new file (JPG), the preview get updated with no issue. But right after I change the title It throw an issue

The website encountered an unexpected error. Try again later.<br><br><em class="placeholder">Drupal\Core\Render\Component\Exception\InvalidComponentException</em>: [image] String value found, but an object is required in <em class="placeholder">Drupal\Core\Theme\Component\ComponentValidator-&gt;validateProps()</em> (line <em class="placeholder">203</em> of <em class="placeholder">core/lib/Drupal/Core/Theme/Component/ComponentValidator.php</em>). <pre class="backtrace">Drupal\Core\Template\ComponentsTwigExtension-&gt;doValidateProps() (Line: 109)

Payload (after changing title field)

{"name":"Icon Card","image":{"alt":"Alternative text ","width":1280,"height":853,"src":"/sites/default/files/2024-09/banner-4887650_1280_1.jpg"},"title":"User-centric experiences","summary":"Create compelling content on more  devices than ever. With tools to build versatile, structured content and integrate with a wide array of digital marketing channels.","randomProp":"random"}

Payload

{"name":"Icon Card","image":"","title":"User","summary":"","randomProp":"random"}

Hi @kisten,

I was able to solve some of the issues

1. [image] String value found, but an object is required

setFormState was settings image to '' (empty string) because the prior state had an entry xb_component_props[9acb612e-748c-4f69-b07e-66b95e5d173e][image][media_library_selection]

2. Textarea

Textarea wasn't getting updated, I found out that the onChange (from InputBehaviors) doesn't get triggered. I updated the code (+ added value because textarea have an entry value on the properties level)

This still doesn't resolve the issue with multiple instances of Card Icon. And another interesting issue, image doesn't get updated in the preview -_-

I'm sorry for not moving everything into the same issue, but as you know you need to have the #3472634 fix just to scroll to Icon Card and I don't think we can rebase on 3 differents issues just to have the ability to test and resolve this one.

Hi !

It looks like we have an issue on Redux level, after I click on the 2nd Card Item. Redux stuck at status: "pending"

UPDATE

once I commented the below in createApi the issue is gone

      transformResponse: processResponseAssets,

Finally, I was able to trace this (well the issue was there all the time "loadjs")

// JS

when you open a 2nd Icon Card, it try to load the same JS libraries that were loaded before. This throw an error, because we have a function that filter the JS array and only load the new JS - and it send an empty string which break loadjs

// CSS

LoadJS throw the below error (not sure why they couldn't add an explanation -_-)

  // throw error if bundle is already defined
  if (bundleId) {
    if (bundleId in bundleIdCache) {
      throw "LoadJS";
    } else {
      bundleIdCache[bundleId] = true;
    }
  }

Source code: https://github.com/kubetail-org/loadjs/blob/main/src/loadjs.js#L248

props are not hanging anymore, but we still need to figure why the Card Item doesn't get updated (Preview)

@boulaffasae Nice detective work! 👏👏

Root cause: ajaxPageState is missing

when you open a 2nd Icon Card, it try to load the same JS libraries that were loaded before. This throw an error, because we have a function that filter the JS array and only load the new JS - and it send an empty string which break loadjs

This points to us missing ajaxPageState support (\Drupal\Core\StackMiddleware\AjaxPageState) in the processResponseAssets function that was added in #3454173: Media Library integration (includes introducing a new main content renderer/`_wrapper_format`). That's how Drupal's \Drupal\Core\Render\MainContent\AjaxRenderer and others handles that, and the ajaxPageState processing happens in \Drupal\Core\Asset\AssetResolver.

Alternative solution

But … your alternative solution sure is intriguing — that'd allow us to not send AJAX page state from the client to server and instead have potentially more cacheable responses… interesting! 😄

Next steps

• 🙏 We should be able to reproduce this using an end-to-end test. Would you be willing to write that, @boulaffasae? 😊
• @bnjmnm, WDYT about @boulaffasae's alternative solution?

Hi @wim leers

I was able to reproduce the issue just by inserting one Icon Card in the existing Two Columns.

I added a simple test, that only check if clicking on that Icon Card will show the right props form / Currently, it show the Image props form instead of the Icon Card props form

Hi @wim leers

I tried getting the libraries and running UrlHelper::compressQueryParameter to compress them so we can attach them to the response:

    if ($libraries = $assets->getLibraries()) {
      $libraries = UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries)));
      $response->headers->set('Attach-Libraries', $libraries);
    }

What do you think ? We should decompress them on the processResponseAssets.ts level, do you think this is possible ?

#16 UrlHelper::compressQueryParameter uses zlib (gzcompress) which is not supported in the browser, so the frontend would also need to install a dependency like https://github.com/imaya/zlib.js/

That approach should be able to solve #3471974: Remove obsolete message about Media Library widget not working when JS aggregation is enabled as well...

Hi @wim leers

I think we will run into a missing piece even after decompressing the libraries / the output will have names likes: core/ajax, ...etc

Redux might not understand those entries.

Can you please direct me into the correct ghin to do ^^

Some context //

This issue is not related to redux (redux show the spinner, because the ajax.js stuck)

When you open a component contextual panel, it try to load JS/CSS dependencies (dialog.js, ...etc)

If two components depend on the same JS, what should happen ?

1/ only load the dependencie one-time, but this will not trigger Drupal.attachBehaviors :/
2/ load the dependencie twice, this will result in a JS file being included two times and will break the click trigger
3/ load the dependecie once, and execute the Drupal.attachBehaviors (something doesn't seem right - the contextual panel stuck at the 1st component props form)

Nice — tests exist now! 👍

If two components depend on the same JS, what should happen ?

1/ only load the dependencie one-time, but this will not trigger Drupal.attachBehaviors :/
2/ load the dependencie twice, this will result in a JS file being included two times and will break the click trigger
3/ load the dependecie once, and execute the Drupal.attachBehaviors (something doesn't seem right - the contextual panel stuck at the 1st component props form)

1. We must manually invoke Drupal.attachBehaviors() — that's fine! :) That's how Drupal's AJAX system works too.
2. 👎
3. This should work fine. That's how Drupal's AJAX system works too.

1+3 are actually already happening — see Drupal.behaviors.jsxAjaxProcess() in ajax.hyperscriptify.js :)

I don't see why @boulaffasae's proposed solution in the MR can't work — i.e. ignore any Attach-Css calls that have already been executed.

Ah … that won't work as soon as we're dealing with aggregated assets! Because then each component with a different set of asset libraries (e.g. A + B + C) than another component but with a different set (e.g. B + C), will cause a different asset URL to be generated, and hence fail.

Conclusion: both the client-side and the server side MUST be updated to use AjaxPageState as explained in #13.

Note: this will likely also solve #3471974: Remove obsolete message about Media Library widget not working when JS aggregation is enabled.

A little update

Textarea + Image fix are no more needed (this was resolved in #3473155)

I will update the PR

HOW TO RESOLVE THE ISSUE MANUALLY

1. Add an Icon Card and upload an Icon
2. Now add an Icon Card (but don't open the Contextual Panel)

Go to web/modules/contrib/experience_builder/src/Render/MainContent/XBEndpointRenderer.php, both response->headers->set('Attach-Css', ... and response->headers->set('Attach-Js', ... should be commented (else XB will load the ajax.js a 2nd time which will result in ajaxCommands being override and we will lose the openDialog function definition, this will break the Media functionnalities for the new Icon Card)

3. Now youn can safely open the Contextual Panel (but don't click the Add Media just yet)

Go to web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php, and update the setAlreadyLoadedLibraries replace the args with an empty array and then you can click Add Media and everything will work fine

      ->setAlreadyLoadedLibraries([])

WHAT NOW

I think @wim leers suggestions will resolve the 1st issue, I will work on that. Then we will need to find out how to resolve the 2nd issue

2nd issue explanation: When Ajax load the necessary JS/CSS for the Media Library widget, it try to skip loaded libraries but then If no JS get loaded Ajax skip also the drupalSettings part :/ (Yes he do - ajax return only $response->getCommands() which concatenate js_assets_footer, js_assets_header, and css_assets what about $attachments['drupalSettings'] = $assets->getSettings())

I was able to reproduce this with the default components that are loaded in XB. It seems that the problem is that you can only use a single component with an image – after highlighting a component, something about the state is off and means that other components with images cannot be edited anymore.

MR!306 solves the issue for me. What was happening is that the (filtered) set of libraries sent to add_js was empty for the second instance of the element, and loadjs() does not call the success callback when asked to do nothing. The fix is to filter the list of libraries *before* trying to send it to add_js.

This could also be considered a bug in core ajax.js in that the promise never resolves if an empty set of libraries is passed in?

I haven't yet got the Cypress tests working locally so I cherry-picked the new test from the previous branch.

Hi @wim leers, I added ajax_page_state as you suggested

        "ajax_page_state[libraries]": drupalSettings.ajaxPageState?.libraries,

I added only libraries (adding theme result in an issue - I don't think we need it)

I added setTimeout (because I found out they use it also for Drupal object + It help trigger the functions inside only after page load = until drupalSettings is defined)

** web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php will decompress the string

and then it's up to XBEndpointRenderer to remove the existing CSS/JS from the Array

I still have an issue, Media Library doesn't open - I think it's related to the #2 issue ('insert' doesn't get triggered in ajax.js)

I'm playing manually again, after uploading an Image in the 1st Icon Card, I add a 2nd Icon Card to the page and before adding a media, I go to web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php:

I comment the code

//    ->setAlreadyLoadedLibraries(isset($ajax_page_state['libraries']) ? explode(',', $ajax_page_state['libraries']) : [])

and I add these at the end

    unset($commands[1]);
    unset($commands[2]);
    return $commands;

this help me load the settings in the Ajax response + Removing the JS/CSS (they were already loaded in the 1st component edit)

and things work just fine.

Now I need to figure out how to do this programmatically

@kristen pol

1. Install Experience Builder, Media, Media Library
2. On /xb/node/1 add a second Image component to the page
3. Click on one of the image components, the form will display
4. Click on the other image component, the form just spins

On the 3472900-fix-empty-js branch this works for me, both forms display as expected - however the second form doesn't work properly, but it does display now.

Remember to npm run build and ddev drush cr after switching branches.

As I concluded at the end of #25, I think the only correct way forward is using ajaxPageState, which @boulaffasae's MR uses, but @longwave's MR does not 😅

So while @longwave's MR might be sufficient to unblock @lauriii's upcoming DrupalCon Barcelona demo recording (yay!), I do think that it makes more sense to continue with @boulaffasae's MR? 🙈

Reviewed @boulaffasae's MR.

Hi everyone, I recorded a demo (Four columns with Cards)

- https://youtu.be/IVyarGnVZOQ

We still have a small issue with Media widget (already addressed in #3473155 https://www.drupal.org/project/experience_builder/issues/3473155 - but they only added a warning about it in console warns "The field media does not yet support updating the preview on change. It will soon.")

I think I have a fix for that, but for now I went with a small trick to skip it (Uploading a new image in the media - Clicking outside the Card - re-click again on the Card and change the title if you want)

This is the small piece of code that I added to web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php

    // Issue #3472900 by kristen pol: XB props form hanging for SDDS components
    $found = false;
    foreach ($commands as $entry) {
      if (isset($entry["command"]) && $entry["command"] === "settings") {
        $found = true; break;
      }
    }
    if (!$found) {
      array_unshift($commands, [
        'command' => 'settings',
        'settings' => $assets->getSettings(),
        'merge' => true,
      ]);
    }

FWIW prop-types.cy.js is now failing in both MRs due to #3469855: Emptying a required value through the UI crashes the app (empty <input>, selecting "None" option in <select> …) when testing the <select> enum and setting the value to "None".

Just two people working on it with slightly different approaches - mine solved exactly the issue in the issue summary but there are wider problems with adding JS when widget forms are loaded in, which the other MR is trying to solve.

I think the issue should be split in two to solve the immediate UI issue here and then fix the bigger Ajax issue separately, but I don't think Wim agrees with this.

Hi @kristen, @longwave

The issue is much bigger than just a CSS/JS loading. @longwace MR will help us build a demo for the upcoming event, and in the meantime I will try to continue with the ajax_page_state approche to resolve the whole issue.

1. Media Library duplicate JS/CSS files, which result in overriding definition (Yes! when Media Library load ajax.js, it automatically erase every definition in dialog.ajax.js - therefore openDialog become undefined and nothing happen on ajax)
2. We can filter existing JS, but then LoadJS library does throw an issue when we pass an empty Array.

-- longwave MR resolve these two

3. Wim suggested that we use ajax_page_state, we need it's logic to skip loading an existing library JS/CSS. But this conflict with Radix state.
4. AjaxResponseAttachmentsProcessor return empty Ajax settings after inserting a Media just because all JS/CSS files are already downloaded.
5. Fied media is not yet supported in Experience Builder (uploading an image might result in it not show up in Preview)

and now it seems like we have new issues with Select too (like we did with Textarea before)

@bnjmnm and I pair-reviewed this to determine next steps.

He's going to combine the excellent work in both MRs (👏 @boulaffasae especially! 😄).

New MR combining the prior solutions is ready.

While manual testing, there's still a strong chance you'll encounter things that don't fully work as expected due to this entire project still being in the "under construction" phase.

If they appear to be problems introduced by the solution in the MR, lets address them in this issue

If they are issues not *specific* to the symptoms in the issue summary and not *introduced* by the MR - first see if there is an existing issue filed and if not file one and no need to even mention it here. The changes in this MR are very beneficial and the more we can streamline communication the faster this will land.

Reviewed! Looks great! There's two bits that I don't understand what they do in there, and I'm hopeful you'll be able to just remove them 🤞

Feeback addressed. Left threads open for anything with an explanation that might require clarification.

RTBC — but please add one example at https://git.drupalcode.org/project/experience_builder/-/merge_requests/3..., that'll help the next person reading that file 😊

but please add one example at https://git.drupalcode.org/project/experience_builder/-/merge_requests/3..., that'll help the next person reading that file

Example added