Ensure uniqueBundleId is unique in LoadJS

Created on 25 July 2024, 7 months ago
Updated 20 September 2024, 5 months ago

Problem/Motivation

In some situations, BigPpe fails to replace content, and a JavaScript error like this appears in the console log:

An error occurred during the execution of the Ajax response: LoadJS
(anonymous) @ ajax.js?v=10.3.1:1143
Promise.catch (async)
Drupal.Ajax.success @ ajax.js?v=10.3.1:1141
processReplacement @ big_pipe.js?v=10.3.1:84
checkMutationAndProcess @ big_pipe.js?v=10.3.1:110
(anonymous) @ big_pipe.js?v=10.3.1:134
processMutations @ big_pipe.js?v=10.3.1:133

We're seeing it with the Flag module, but it's been reported with other modules, too:

Doing some debugging, I found that the "LoadJS" exception is only thrown when loadjs tries to load a CSS/JS asset that's already been loaded:

https://github.com/kubetail-org/loadjs/blob/main/src/loadjs.js#L245

There is this code in ajax.js for the add_css and add_js Ajax commands:

add_css(ajax, response, status) {
  const allUniqueBundleIds = response.data.map(function (style) {
    const uniqueBundleId = style.href + ajax.instanceIndex;
    // Force file to load as a CSS stylesheet using 'css!' flag.
    loadjs(`css!${style.href}`, uniqueBundleId, {
add_js(ajax, response, status) {
  const parentEl = document.querySelector(response.selector || 'body');
  const settings = ajax.settings || drupalSettings;
  const allUniqueBundleIds = response.data.map((script) => {
    // loadjs requires a unique ID, and an AJAX instance's `instanceIndex`
    // is guaranteed to be unique.
    // @see Drupal.behaviors.AJAX.detach
    const uniqueBundleId = script.src + ajax.instanceIndex;
    loadjs(script.src, uniqueBundleId, {

Per the comment in add_js, it's assuming that ajax.instanceIndex is unique and will therefor make uniqueBundleId unique. That is not actually the case here. On our site, we're seeing 3 calls to add_css and 1 to add_js, each with ajax.instanceIndex set to 0. Two of those calls are trying to load the same CSS file from Flag: /modules/contrib/flag/css/flag-link.css?sh4z48. That second call fails because loadjs throws an exception. This prevents BigPipe from being able to properly swap out the content. 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?

Steps to reproduce

We can reproduce this error with just the Flag module:

  1. Install site with Flag module.
  2. Create Flag that uses "Link type" of "Confirm Form".
  3. Create 3-4 content items that will have the Flag.
  4. Go to /node page or create a View that shows that flaggable content and go to that View's page.
  5. See that all "Flag this item" links after the first one are missing.
  6. See the "An error occurred during the execution of the Ajax response: LoadJS" in the JavaScript console log once for every item shown after the first one.

Proposed resolution

In those other issues referenced above, the fix (where there is one) was to try to prevent the second add_js/add_css call. I don't think that's the best solution, though. Per that comment in ajax.js, it seems that it's not actually concerned with loading duplicates-- it's happy to load the same resources multiple times and assumes that using ajax.instanceIndex will prevent any errors. If this code thought it would be a real problem, it would keep track of the resources loaded. I don't think that's needed. And, it looks like the browser will make only one request for a given resource regardless of how many <link>/<script> tags for it are added.

I think that the real solution is to make sure the uniqueBundleId is actually always unique. We can do this by just keeping our own counter that we increment each time we add a resource and use that instead of ajax.instanceIndex.

Alternate solution: The loadjs library provides an `isDefined()` method to check whether a bundle ID has already been defined. We can use that method to gate whether to load the script/css again. See loadjs documentation.

Remaining tasks

  1. Fix ajax.js code.
  2. Add tests.
  3. Merge fix.
πŸ› Bug report
Status

Needs review

Version

11.0 πŸ”₯

Component
BigPipe  β†’

Last updated 7 days ago

Created by

πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jrb
  • Status changed to Needs review 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA

    Here is a patch that adds a counter and uses that to make sure that uniqueBundleId is always unique.

    This fixes our issue with Flag and should fix the other referenced issues above.

  • πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA
  • Status changed to Needs work 7 months ago
  • 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.)

  • Status changed to Needs review 7 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone
  • Status changed to Needs work 7 months ago
  • 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.

  • Pipeline finished with Success
    7 months ago
    Total: 475s
    #234896
  • Status changed to Needs review 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States jrb Raleigh-Durham Area, NC, USA

    Here's a patch from the MR.

    Note that this patch is for 11.x while the patch in #2 above will apply to 10.x.

  • Status changed to Needs work 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

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

  • πŸ‡΅πŸ‡±Poland michal_liszka

    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"
    >
    
  • πŸ‡ͺπŸ‡ΈSpain espurnes

    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.

  • πŸ‡³πŸ‡±Netherlands spokje

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

    Patch #2 worked for me.

  • Pipeline finished with Canceled
    7 months ago
    Total: 126s
    #251942
  • Pipeline finished with Success
    7 months ago
    Total: 688s
    #251943
  • Status changed to Needs review 7 months ago
  • πŸ‡³πŸ‡±Netherlands spokje

    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.

  • First commit to issue fork.
  • 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.

  • godotislate β†’ changed the visibility of the branch 3463875-bigpipe-loadjs-errors to hidden.

  • Pipeline finished with Success
    6 months ago
    Total: 490132s
    #252131
  • Status changed to RTBC 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Believe adding the isDefined check makes sense.

  • Status changed to Needs work 6 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone

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

  • Pipeline finished with Success
    6 months ago
    Total: 543s
    #264504
  • Pipeline finished with Canceled
    6 months ago
    Total: 66s
    #264534
  • Pipeline finished with Failed
    6 months ago
    Total: 538s
    #264523
  • Pipeline finished with Failed
    6 months ago
    Total: 596s
    #264535
  • Pipeline finished with Success
    6 months ago
    #264545
  • Pipeline finished with Success
    6 months ago
    Total: 652s
    #264556
  • Status changed to Needs review 6 months ago
  • πŸ‡³πŸ‡±Netherlands spokje

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

  • Status changed to RTBC 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Believe additional feedback has been addressed.

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

  • Status changed to Needs work 6 months ago
  • πŸ‡«πŸ‡·France nod_ Lille

    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 still 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?

  • Status changed to Needs review 5 months ago
  • πŸ‡³πŸ‡±Netherlands spokje
  • πŸ‡ΊπŸ‡ΈUnited States pcate

    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.

  • πŸ‡«πŸ‡·France nod_ Lille

    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 β†’

  • πŸ‡³πŸ‡±Netherlands spokje

    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.

  • πŸ‡«πŸ‡·France nod_ Lille

    yeah i'd rather have this in than not in

    • nod_ β†’ committed 9b977fc3 on 10.3.x
      Issue #3463875 by spokje, jrb, godotislate, smustgrave: Ensure...
    • nod_ β†’ committed b60151a2 on 10.4.x
      Issue #3463875 by spokje, jrb, godotislate, smustgrave: Ensure...
    • nod_ β†’ committed 0b0d880c on 11.0.x
      Issue #3463875 by spokje, jrb, godotislate, smustgrave: Ensure...
    • nod_ β†’ committed d44fb628 on 11.x
      Issue #3463875 by spokje, jrb, godotislate, smustgrave: Ensure...
  • πŸ‡«πŸ‡·France nod_ Lille

    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!

  • πŸ‡¦πŸ‡ΊAustralia interlated

    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';
    }
    
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡¨πŸ‡¦Canada ambient.impact Toronto

    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.

Production build 0.71.5 2024