Ensure uniqueBundleId is unique in LoadJS

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Thanks for reporting, as a bug can we get a test case showing the issue

Despite applying patch #2 for Drupal 10, the problem persists when request the controller for a response via AJAX dialogs.
The first request executes correctly, and the response is accurate on the second attempt. However, the same error continues to occur:
An error occurred during the execution of the Ajax response: LoadJS

Code snippet:

<a
   href="{{ url('controller_route_name') }}"
   class="use-ajax"
   data-dialog-options="{{ dialog_options|json_encode() }}"
   data-dialog-type="modal"
   data-ajax-http-method="GET"
>

After updating to Drupal version 10.3.1 (I was before at version 10.2.4), the "An error occurred during the execution of the Ajax response: LoadJS" error is generated on my custom code after CTAs with ajax actions that should replace page content. Big pipe is not enabled. So it should be related with the way I perform the ajax calls (or the addition of the libraries) and the way drupal 10.3 processed them.

Applying patch #2 solves the issue.

I've stumbled over this when trying to open a configuration on /admin/config/development/configuration more than once, even in the Claro theme.
Might make it easier to reproduce in core only for testing.

Patch #2 worked for me.

MR now has a test. Tried to do this in \Drupal\Tests\system\FunctionalJavascript\ModalRendererTest but couldn't get the error there, maybe/probably due to the lack of JS/CSS on that page?

So I decided to use a bit of \Drupal\Tests\config\Functional\ConfigImportUITest in a new FunctionalJavascriptTest.
Not great, not fast, but we need JS testing for this and this was the only place in core I could reproduce the error.

MR passes, test-only fails where it's expected to fail.

Looking at the documentation for loadjs, there is a isDefined() method that checks for existing bundle IDs, so I think the counter might be unnecessary altogether. I created a new MR 9184 based on the original MR, and added isDefined() checks. It passes all tests, including the new one from #13. Git history of the previous MR is intact, so this approach can be easily reverted if the other way is preferred.

Believe adding the isDefined check makes sense.

Trying for a descriptive of what is being fixed. I also left 2 comments in the MR about documentation that need attention.

Thanks @quietone, applied your suggestion and added a comment.

Believe additional feedback has been addressed.

Hoping eventually can help close resolved threads but can't help there either :(

I'm not sure why ajax.instanceIndex is 0 in each case, but it is. Maybe there are some cases where Drupal.ajax is called multiple times, resetting Drupal.ajax.instances?

In the case of bigpipe we reuse the same ajax object to execute all the ajax commands so that makes sense. What is surprising is why the same file is added in two different ajax requests, it means the ajax page state is not updated for some reason.

I tried with layout_builder_browser and could replicate the problem without removing the ajaxInstanceIndex from the bundle ID. Why is the ajaxIndex removed from the bundle Id?

I tried with layout_builder_browser and could replicate the problem without removing the ajaxInstanceIndex from the bundle ID. Why is the ajaxIndex removed from the bundle Id?

Replied in MR comment, but the idea is that the same resource is being prevented from being loaded twice by using loadJs.isDefined() to check whether the bundle ID already exists. The counter then should become unnecessary.

Is the problem still replicable with layout_builder_browser even with the latest MR? If so, can you provide steps to reproduce?

I just ran into this error when loading a view block on Drupal 10.3.5. Layout Builder is not used on the site, the block was added via the regular block layout page. The big pipe placeholder element is rendered on the page but the error prevents it being replaced with the actual content.

The MR resolved the error and loaded the block fine.

Updated the IS with the solution implemented the MR.

Ok, Happy with the JS changes.

For the test, I understand but i'm not comfortable using a sideeffect of the config UI to test an ajax regression. Can we make a more explicit test by trying to load the same file twice through add_js and add_css? It's a missing test so I guess that's why you had to create a new one: #3301769: Add test for the new add_js command

For the test, I understand but i'm not comfortable using a sideeffect of the config UI to test an ajax regression. Can we make a more explicit test by trying to load the same file twice through add_js and add_css? It's a missing test so I guess that's why you had to create a new one: #3301769: Add test for the new add_js command

I can understand this isn't a nice direct test, but it was the only way I could reproduce it in core.

I'm a tad confused why add_js/add_css could reproduce the issue when the commands are new, whilst I think the config UI didn't change that much in recent time?

Anyway, I'm sure someone with actual knowledge will pick this up in the end.

yeah i'd rather have this in than not in

Committed and pushed d44fb628e45 to 11.x and 0b0d880cbfd to 11.0.x and b60151a2656 to 10.4.x and 9b977fc3f84 to 10.3.x. Thanks!

This fixed a problem with ajax modals

- Install layout builder, linked_field
- Link the title in layout builder.
- install the code below in the custom theme
- clear cache
- Open modal once
- Close modal
- Open again.

A cryptic 'LoadJS' error message is returned on the console. Line 1143 ajax.js.

function tn_preprocess_field(&$variables) {
  $element = $variables['element'];
  if ($element['#field_name'] == 'title' && $element['#bundle'] == 'course' && $element['#view_mode'] == '_custom') {
    tn_add_modal_attributes($variables);
  }
}

function tn_add_modal_attributes(&$variables) {
  $variables['items'][0]['content']['#options']['attributes']['class'][] = 'use-ajax';
  $variables['items'][0]['content']['#options']['attributes']['data-dialog-type'][] = 'modal';
  $variables['items'][0]['content']['#options']['attributes']['data-dialog-options'][] = '{"width":"80vw","height":"100%", "dialogClass": "course-dialog", "position": {"my": "right top", "at": "right top"}, "show":"slideDown","hide":"slideUp"}';
  $variables['items'][0]['content']['#options']['attributes']['data-ajax-focus'][] = 'none';
}

Ah, I see I'm not the only one that was utterly confused by the cryptic "LoadJS" error. It turns out the library did this to keep the size down, which I personally feel was a bad move because it's so confusing and non-helpful. I opened a pull request to change it, but since they still want to keep the size down, they're only willing to throw the bundle ID as the error message rather than a fully descriptive thing that says why it was thrown. Glad to see this is being handled on the Drupal side.