Ajax.js commands stuck when adding JS with nomodule attribute

Created on 19 January 2023, almost 2 years ago

Updated 14 February 2023, almost 2 years ago

Problem/Motivation

(Note: this is a regression that affects 9.5+ and 10+)

When loading a script that has the nomodule attribute via ajax.js, the command execution will get stuck.

With https://www.drupal.org/node/3293812 → , Drupal introduced a new add_js command. The implementation of the add_js command uses the loadjs library. The logic of this command adds the attributes defined for the script, for example in a xxx.libraries.yml file, before loadjs attempts to load the file.

If one of those attributes is nomodule (for example for a polyfill dependency), then the success or error callbacks from loadjs will never be called because the browser (any recent one) completely ignores the script and will not fire any onload, onerror or onbeforeload event that the loadjs listens to for that file.

That results in the counter used by loadjs to determine when to call the success/error callbacks, to not be decremented for this script and thus the callbacks are never called and the ajax.js add_js command promise is not resolved etc.

This is a regression from < 9.5 which had a different mechanism to load the javascript dependencies via ajax.

Steps to reproduce

The following code illustrates the issue at the loadjs level with a stripped down version of the logic from the add_js logic from the ajax.js file. The "success" callback is never called. Without the scriptEl.setAttribute('nomodule', ''); the script is loaded and the success callback is called.

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <title>Test Load JS</title>
</head>
<body>
  <script src="https://cdn.jsdelivr.net/npm/loadjs@4.2.0/dist/loadjs.min.js"></script>
  <script>
    loadjs(['https://cdn.jsdelivr.net/npm/css-vars-ponyfill@2.4.8/dist/css-vars-ponyfill.min.js'], 'css-vars-ponyfill', {
      async: false,
      before: function(path, scriptEl) {
        console.log(path);
        scriptEl.setAttribute('nomodule', '');
        return false;
      }
    });

    loadjs.ready('css-vars-ponyfill', {
      success: function() {
        console.log('success');
      },
      error: function(depsNotFound) {
        console.log(depsNotFound);
      },
    });
  </script>
</body>
</html>

The effect can be seen on a fresh Drupal install following those steps (just for the sake of illustrating the problem):

Spin up a new Drupal 9.5.x site
Enable the "Block" module
Edit the "core.libraries.yml" file and under drupal.collapse, replace the line misc/details-aria.js: {} with misc/details-aria.js: { attributes: { nomodule: true } }
Clear the cache
Go to /admin/structure/block, click a "Place block" button, in the popup, chose a block and click "Add block"
With the nomodule change from (3) above, the content of the popup will not change to the block settings because the ajax command to update it will not be executed
Just to confirm, remove the change from (3), refresh the cache and try again and this time the popup's content will be updated properly to go to the next step to add the block

Proposed resolution

I'm not sure at which level (Drupal or Loadjs) the issue should be solved. It's not really a problem in loadjs (by default, it doesn't handle attributes to add to a script, those are added by the Drupal ajax.js logic).

I think the remediation could be in the ajax.js add_js logic with 2 options:

1. Do not load a file with the nomodule attribute
2. Or, do not add the nomodule attribute

All recent browsers support the nomodule attribute. The versions supporting this attribute match the Drupal browser requirements → so option 1 would probably be fine.

Note: skipping scripts with the nomodule attribute is also what jquery seems to do: https://github.com/jquery/jquery/pull/4282

Here's the issue in the loadjs repo, in case this should be fixed there: https://github.com/muicss/loadjs/issues/108