Issue #3526267 by fathershawn, nod_, catch, jrockowitz, richgerdes,...

Issue #3526267 by fathershawn, nod_, catch, jrockowitz, richgerdes, godotislate, larowlan: Remove core/drupal.ajax dependency from  big_pipe/big_pipe

core/core.libraries.yml

@@ -617,6 +617,7 @@ drupal.form:
 drupal.htmx:
   version: VERSION
   js:
+    misc/htmx/htmx-utils.js: {}
     misc/htmx/htmx-assets.js: {}
     misc/htmx/htmx-behaviors.js: {}
   dependencies:

core/misc/htmx/htmx-assets.js

@@ -7,10 +7,7 @@
  * page.
  */
 
-(function (Drupal, drupalSettings, loadjs, htmx) {
-  // Disable htmx loading of script tags since we're handling it.
-  htmx.config.allowScriptTags = false;
-
+(function (Drupal, drupalSettings, htmx) {
   /**
    * Used to hold the loadjs promise.
    *
@@ -22,46 +19,12 @@
   const requestAssetsLoaded = new WeakMap();
 
   /**
-   * Helper function to merge two objects recursively.
-   *
-   * @param current
-   *   The object to receive the merged values.
-   * @param sources
-   *   The objects to merge into current.
-   *
-   * @return object
-   *   The merged object.
    *
-   * @see https://youmightnotneedjquery.com/#deep_extend
    */
-  function mergeSettings(current, ...sources) {
-    if (!current) {
-      return {};
-    }
-
-    sources
-      .filter((obj) => Boolean(obj))
-      .forEach((obj) => {
-        Object.entries(obj).forEach(([key, value]) => {
-          switch (Object.prototype.toString.call(value)) {
-            case '[object Object]':
-              current[key] = current[key] || {};
-              current[key] = mergeSettings(current[key], value);
-              break;
-
-            case '[object Array]':
-              current[key] = mergeSettings(new Array(value.length), value);
-              break;
-
-            default:
-              current[key] = value;
-          }
-        });
+  htmx.on('htmx:beforeRequest', ({ detail }) => {
+    requestAssetsLoaded.set(detail.xhr, Promise.resolve());
   });
 
-    return current;
-  }
-
   /**
    * Send the current ajax page state with each request.
    *
@@ -94,6 +57,10 @@
     // Custom event to detach behaviors.
     htmx.trigger(detail.elt, 'htmx:drupal:unload');
 
+    if (!detail.xhr) {
+      return;
+    }
+
     // We need to parse the response to find all the assets to load.
     // htmx cleans up too many things to be able to rely on their dom fragment.
     let responseHTML = Document.parseHTMLUnsafe(detail.serverResponse);
@@ -103,73 +70,49 @@
     const settingsElement = responseHTML.querySelector(
       ':is(head, body) > script[type="application/json"][data-drupal-selector="drupal-settings-json"]',
     );
+    // Remove so that HTML doesn't add this during swap.
+    settingsElement?.remove();
+
     if (settingsElement !== null) {
-      mergeSettings(drupalSettings, JSON.parse(settingsElement.textContent));
+      Drupal.htmx.mergeSettings(
+        drupalSettings,
+        JSON.parse(settingsElement.textContent),
+      );
     }
 
     // Load all assets files. We sent ajax_page_state in the request so this is only the diff with the current page.
-    const assetsTags = responseHTML.querySelectorAll(
+    const assetsElements = responseHTML.querySelectorAll(
       'link[rel="stylesheet"][href], script[src]',
     );
-    const bundleIds = Array.from(assetsTags)
-      .filter(({ href, src }) => !loadjs.isDefined(href ?? src))
-      .map(({ href, src, type, attributes }) => {
-        const bundleId = href ?? src;
-        let prefix = 'css!';
-        if (src) {
-          prefix = type === 'module' ? 'module!' : 'js!';
-        }
-
-        loadjs(prefix + bundleId, bundleId, {
-          // JS files are loaded in order, so this needs to be false when 'src'
-          // is defined.
-          async: !src,
-          // Copy asset tag attributes to the new element.
-          before(path, element) {
-            // This allows all attributes to be added, like defer, async and
-            // crossorigin.
-            Object.values(attributes).forEach((attr) => {
-              element.setAttribute(attr.name, attr.value);
+    // Remove all assets from the serverResponse where we handle the loading.
+    assetsElements.forEach((element) => element.remove());
+
+    // Transform the data from the DOM into an ajax command like format.
+    const data = Array.from(assetsElements).map(({ attributes }) => {
+      const attrs = {};
+      Object.values(attributes).forEach(({ name, value }) => {
+        attrs[name] = value;
       });
-          },
+      return attrs;
     });
 
-        return bundleId;
-      });
+    // The response is the whole page without the assets we handle with loadjs.
+    detail.serverResponse = responseHTML.documentElement.outerHTML;
 
     // Helps with memory management.
     responseHTML = null;
 
-    // Nothing to load, we resolve the promise right away.
-    let assetsLoaded = Promise.resolve();
-    // If there are assets to load, use loadjs to manage this process.
-    if (bundleIds.length) {
-      // Trigger the event once all the dependencies have loaded.
-      assetsLoaded = new Promise((resolve, reject) => {
-        loadjs.ready(bundleIds, {
-          success: resolve,
-          error(depsNotFound) {
-            const message = Drupal.t(
-              `The following files could not be loaded: @dependencies`,
-              { '@dependencies': depsNotFound.join(', ') },
-            );
-            reject(message);
-          },
-        });
-      });
-    }
-
-    requestAssetsLoaded.set(detail.xhr, assetsLoaded);
+    requestAssetsLoaded.get(detail.xhr).then(() => Drupal.htmx.addAssets(data));
   });
 
   // Trigger the Drupal processing once all assets have been loaded.
   // @see https://htmx.org/events/#htmx:afterSettle
   htmx.on('htmx:afterSettle', ({ detail }) => {
-    requestAssetsLoaded.get(detail.xhr).then(() => {
+    (requestAssetsLoaded.get(detail.xhr) || Promise.resolve()).then(() => {
       // Some HTMX swaps put the incoming element before or after detail.elt.
       htmx.trigger(detail.elt.parentNode, 'htmx:drupal:load');
       // This should be automatic but don't wait for the garbage collector.
       requestAssetsLoaded.delete(detail.xhr);
     });
   });
-})(Drupal, drupalSettings, loadjs, htmx);
+})(Drupal, drupalSettings, htmx);

core/misc/htmx/htmx-utils.js

+/**
+ * @file
+ * Connect Drupal.behaviors to htmx inserted content.
+ */
+(function (Drupal, htmx, drupalSettings, loadjs) {
+  /**
+   * Namespace for htmx utilities.
+   */
+  Drupal.htmx = {
+    /**
+     * Helper function to merge two objects recursively.
+     *
+     * @param current
+     *   The object to receive the merged values.
+     * @param sources
+     *   The objects to merge into current.
+     *
+     * @return object
+     *   The merged object.
+     *
+     * @see https://youmightnotneedjquery.com/#deep_extend
+     */
+    mergeSettings(current, ...sources) {
+      if (!current) {
+        return {};
+      }
+
+      sources
+        .filter((obj) => Boolean(obj))
+        .forEach((obj) => {
+          Object.entries(obj).forEach(([key, value]) => {
+            switch (Object.prototype.toString.call(value)) {
+              case '[object Object]':
+                current[key] = current[key] || {};
+                current[key] = Drupal.htmx.mergeSettings(current[key], value);
+                break;
+
+              case '[object Array]':
+                current[key] = Drupal.htmx.mergeSettings(
+                  new Array(value.length),
+                  value,
+                );
+                break;
+
+              default:
+                current[key] = value;
+            }
+          });
+        });
+
+      return current;
+    },
+
+    /**
+     *
+     * @param {array} data
+     *
+     * @return {Promise}
+     */
+    addAssets(data) {
+      const bundleIds = data
+        .filter(({ href, src }) => !loadjs.isDefined(href ?? src))
+        .map(({ href, src, type, ...attributes }) => {
+          const bundleId = href ?? src;
+          let prefix = 'css!';
+          if (src) {
+            prefix = type === 'module' ? 'module!' : '';
+          }
+
+          loadjs(prefix + bundleId, bundleId, {
+            // JS files are loaded in order, so this needs to be false when 'src'
+            // is defined.
+            async: !src,
+            // Copy asset tag attributes to the new element.
+            before(path, element) {
+              // This allows all attributes to be added, like defer, async and
+              // crossorigin.
+              Object.entries(attributes).forEach(([name, value]) => {
+                element.setAttribute(name, value);
+              });
+            },
+          });
+
+          return bundleId;
+        });
+
+      // Nothing to load, we resolve the promise right away.
+      let assetsLoaded = Promise.resolve();
+      // If there are assets to load, use loadjs to manage this process.
+      if (bundleIds.length) {
+        // Trigger the event once all the dependencies have loaded.
+        assetsLoaded = new Promise((resolve, reject) => {
+          loadjs.ready(bundleIds, {
+            success: resolve,
+            error(depsNotFound) {
+              const message = Drupal.t(
+                `The following files could not be loaded: @dependencies`,
+                { '@dependencies': depsNotFound.join(', ') },
+              );
+              reject(message);
+            },
+          });
+        });
+      }
+
+      return assetsLoaded;
+    },
+  };
+})(Drupal, htmx, drupalSettings, loadjs);

core/modules/big_pipe/big_pipe.libraries.yml

 big_pipe:
   version: VERSION
   js:
+    js/big_pipe.commands.js: {}
     js/big_pipe.js: {}
   drupalSettings:
     bigPipePlaceholderIds: []
   dependencies:
-    - core/drupal.ajax
+    - core/htmx
+    - core/drupal
+    - core/drupal.htmx
     - core/drupalSettings
+    - core/loadjs
+    - core/drupal.message

core/modules/big_pipe/js/big_pipe.commands.js

+((Drupal, drupalSettings, htmx) => {
+  /**
+   * Holds helpers for big pipe processing.
+   *
+   * @namespace
+   */
+  Drupal.bigPipe = {};
+  /**
+   * Helper method to make sure commands are executed in sequence.
+   *
+   * @param {Array} response
+   *   Drupal Ajax response.
+   * @param {number} status
+   *   XMLHttpRequest status.
+   *
+   * @return {Promise}
+   *  The promise that will resolve once all commands have finished executing.
+   */
+  Drupal.bigPipe.commandExecutionQueue = function (response, status) {
+    const ajaxCommands = Drupal.bigPipe.commands;
+    return Object.keys(response || {}).reduce(
+      // Add all commands to a single execution queue.
+      (executionQueue, key) =>
+        executionQueue.then(() => {
+          const { command } = response[key];
+          if (command && ajaxCommands[command]) {
+            // When a command returns a promise, the remaining commands will not
+            // execute until that promise has been fulfilled. This is typically
+            // used to ensure JavaScript files added via the 'add_js' command
+            // have loaded before subsequent commands execute.
+            return ajaxCommands[command](response[key], status);
+          }
+        }),
+      Promise.resolve(),
+    );
+  };
+
+  /**
+   * Implementation of Drupal ajax commands with htmx.
+   *
+   * @type {object}
+   */
+  Drupal.bigPipe.commands = {
+    /**
+     * Command to insert new content into the DOM.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {string} response.data
+     *   The new HTML.
+     * @param {string} [response.method]
+     *   The jQuery DOM manipulation method to be used.
+     * @param {string} [response.selector]
+     *   An optional selector string.
+     */
+    insert({ data, method, selector }) {
+      const target = htmx.find(selector);
+
+      // Detach behaviors.
+      htmx.trigger(target, 'htmx:drupal:unload');
+
+      // Map jQuery manipulation methods to the DOM equivalent.
+      const styleMap = {
+        replaceWith: 'outerHTML',
+        html: 'innerHTML',
+        before: 'beforebegin',
+        prepend: 'afterbegin',
+        append: 'beforeend',
+        after: 'afterend',
+      };
+
+      // Make the actual swap and initialize everything.
+      htmx.swap(target, data, {
+        swapStyle: styleMap[method] || 'outerHTML',
+      });
+    },
+
+    /**
+     * Command to set the window.location, redirecting the browser.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {string} response.url
+     *   The URL to redirect to.
+     */
+    redirect({ url }) {
+      window.location = url;
+    },
+
+    /**
+     * Command to set the settings used for other commands in this response.
+     *
+     * This method will also remove expired `drupalSettings.ajax` settings.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {boolean} response.merge
+     *   Determines whether the additional settings should be merged to the
+     *   global settings.
+     * @param {object} response.settings
+     *   Contains additional settings to add to the global settings.
+     */
+    settings({ merge, settings }) {
+      if (merge) {
+        Drupal.htmx.mergeSettings(drupalSettings, settings);
+      }
+    },
+
+    /**
+     * Command to add css.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {object[]} response.data
+     *   An array of styles to be added.
+     */
+    add_css({ data }) {
+      return Drupal.htmx.addAssets(data);
+    },
+
+    /**
+     * Command to add a message to the message area.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {string} response.messageWrapperQuerySelector
+     *   The zone where to add the message. If null, the default will be used.
+     * @param {string} response.message
+     *   The message text.
+     * @param {string} response.messageOptions
+     *   The options argument for Drupal.Message().add().
+     * @param {boolean} response.clearPrevious
+     *   If true, clear previous messages.
+     */
+    message({
+      message,
+      messageOptions,
+      messageWrapperQuerySelector,
+      clearPrevious,
+    }) {
+      const messages = new Drupal.Message(
+        document.querySelector(messageWrapperQuerySelector),
+      );
+      if (clearPrevious) {
+        messages.clear();
+      }
+      messages.add(message, messageOptions);
+    },
+
+    /**
+     * Command to add JS.
+     *
+     * @param {object} response
+     *   The response from the Ajax request.
+     * @param {Array} response.data
+     *   An array of objects of script attributes.
+     */
+    add_js({ data }) {
+      return Drupal.htmx.addAssets(data).then(() => {
+        htmx.trigger(document.body, 'htmx:drupal:load');
+      });
+    },
+  };
+})(Drupal, drupalSettings, htmx);

core/modules/big_pipe/js/big_pipe.js

@@ -11,21 +11,6 @@
    */
   const replacementsSelector = `script[data-big-pipe-replacement-for-placeholder-with-id]`;
 
-  /**
-   * Ajax object that will process all the BigPipe responses.
-   *
-   * Create a Drupal.Ajax object without associating an element, a progress
-   * indicator or a URL.
-   *
-   * @type {Drupal.Ajax}
-   */
-  const ajaxObject = Drupal.ajax({
-    url: '',
-    base: false,
-    element: false,
-    progress: false,
-  });
-
   /**
    * Maps textContent of <script type="application/vnd.drupal-ajax"> to an AJAX
    * response.
@@ -81,7 +66,7 @@
 
     // Then, simulate an AJAX response having arrived, and let the Ajax system
     // handle it.
-    ajaxObject.success(response, 'success');
+    Drupal.bigPipe.commandExecutionQueue(response, 'success');
   }
 
   /**
@@ -177,7 +162,5 @@
     if (mutations.length) {
       processMutations(mutations);
     }
-    // No more mutations will be processed, remove the leftover Ajax object.
-    Drupal.ajax.instances[ajaxObject.instanceIndex] = null;
   });
 })(Drupal, drupalSettings);

core/modules/big_pipe/tests/modules/big_pipe_regression_test/big_pipe_regression_test.routing.yml

@@ -26,3 +26,10 @@ big_pipe_test_multiple_replacements:
     _controller: '\Drupal\big_pipe_regression_test\BigPipeRegressionTestController::multipleReplacements'
   requirements:
     _access: 'TRUE'
+
+big_pipe_regression_test.inline_script:
+  path: /big_pipe_regression_inline_script
+  defaults:
+    _controller: '\Drupal\big_pipe_regression_test\BigPipeRegressionTestController::inlineScriptContent'
+  requirements:
+    _access: 'TRUE'

core/modules/big_pipe/tests/modules/big_pipe_regression_test/src/BigPipeRegressionTestController.php

@@ -71,6 +71,16 @@ public function multipleReplacements() {
     return $build;
   }
 
+  /**
+   * A page with an inline script.
+   */
+  public function inlineScriptContent(): array {
+    return [
+      '#lazy_builder' => [static::class . '::inlineScript', []],
+      '#create_placeholder' => TRUE,
+    ];
+  }
+
   /**
    * Renders large content.
    *
@@ -111,11 +121,26 @@ public static function renderRandomSentence(int $length): array {
     return ['#cache' => ['max-age' => 0], '#markup' => (new Random())->sentences($length)];
   }
 
+  /**
+   * Renders an inline script element between other markup tags.
+   *
+   * @return array
+   *   Render array.
+   */
+  public static function inlineScript(): array {
+    return [
+      '#cache' => ['max-age' => 0],
+      '#markup' => BigPipeMarkup::create(
+        '<div class="container-before">First</div><script>document.body.classList.add("inline-script-fires");</script><div class="container-after">Second</div>'
+      ),
+    ];
+  }
+
   /**
    * {@inheritdoc}
    */
   public static function trustedCallbacks() {
-    return ['currentTime', 'largeContentBuilder', 'renderRandomSentence'];
+    return ['currentTime', 'largeContentBuilder', 'renderRandomSentence', 'inlineScript'];
   }
 
 }

core/modules/big_pipe/tests/src/FunctionalJavascript/BigPipeRegressionTest.php

@@ -53,11 +53,10 @@ public function testMultipleClosingBodies_2678662(): void {
     $this->drupalLogin($this->drupalCreateUser());
     $this->drupalGet(Url::fromRoute('big_pipe_regression_test.2678662'));
 
-    // Confirm that AJAX behaviors were instantiated, if not, this points to a
-    // JavaScript syntax error and the JS variable has the appropriate content.
+    // Confirm that the JS variable has the appropriate content.
     $javascript = <<<JS
     (function(){
-      return Object.keys(Drupal.ajax.instances).length > 0 && hitsTheFloor === "</body>";
+      return hitsTheFloor === "</body>";
     }())
 JS;
     $this->assertJsCondition($javascript);
@@ -126,6 +125,7 @@ public function testPlaceholderHtmlEdgeCases(): void {
     $this->doTestPlaceholderInParagraph_2802923();
     $this->doTestBigPipeLargeContent();
     $this->doTestMultipleReplacements();
+    $this->doInlineScriptTest();
   }
 
   /**
@@ -184,4 +184,27 @@ protected function doTestMultipleReplacements(): void {
     $this->assertCount(BigPipeRegressionTestController::PLACEHOLDER_COUNT + 1, $this->getSession()->getPage()->findAll('css', 'script[data-big-pipe-replacement-for-placeholder-with-id]'));
   }
 
+  /**
+   * Tests for the correct replacement of a chunk with inline javascript.
+   *
+   * Parsing a <script> tag causes a mutation to be reported because it
+   * triggers a micro task checkpoint.  Watching for template tags directly
+   * results in early mutation reporting, and the trailing markup is not yet
+   * parsed when the template is processed.
+   *
+   * Also, chunk markup needs to not be double escaped.  We encountered both
+   * regressions in https://www.drupal.org/project/drupal/issues/3526267.
+   */
+  protected function doInlineScriptTest(): void {
+    $user = $this->drupalCreateUser();
+    $this->drupalLogin($user);
+    $assert_session = $this->assertSession();
+
+    $this->drupalGet(Url::fromRoute('big_pipe_regression_test.inline_script'));
+    $this->assertNotNull($assert_session->waitForElement('css', 'script[data-big-pipe-event="stop"]'));
+    $assert_session->elementExists('css', 'div.container-before');
+    $assert_session->elementExists('css', 'body.inline-script-fires');
+    $assert_session->elementExists('css', 'div.container-after');
+  }
+
 }

core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php

@@ -48,9 +48,9 @@ public function testFrontAndRecipesPagesAuthenticated(): void {
 
     $expected = [
       'ScriptCount' => 3,
-      'ScriptBytes' => 170500,
+      'ScriptBytes' => 253948,
       'StylesheetCount' => 5,
-      'StylesheetBytes' => 85000,
+      'StylesheetBytes' => 82672,
     ];
     $this->assertMetrics($expected, $performance_data);
   }
@@ -69,7 +69,7 @@ public function testFrontAndRecipesPagesEditor(): void {
     }, 'umamiFrontAndRecipePagesEditor');
     $expected = [
       'ScriptCount' => 5,
-      'ScriptBytes' => 335003,
+      'ScriptBytes' => 446101,
       'StylesheetCount' => 5,
       'StylesheetBytes' => 205100,
     ];

core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php

@@ -64,10 +64,10 @@ public function testFrontPageAuthenticatedWarmCache(): void {
       'CacheDeleteCount' => 0,
       'CacheTagInvalidationCount' => 0,
       'CacheTagLookupQueryCount' => 5,
-      'ScriptCount' => 2,
-      'ScriptBytes' => 123850,
+      'ScriptCount' => 1,
+      'ScriptBytes' => 73031,
       'StylesheetCount' => 2,
-      'StylesheetBytes' => 41950,
+      'StylesheetBytes' => 39163,
     ];
     $this->assertMetrics($expected, $performance_data);
   }