From aa0ab207c65571cd09e08bc42a4801a678447db4 Mon Sep 17 00:00:00 2001
From: Matei Stanca <i@ambientimpact.com>
Date: Tue, 1 Oct 2024 13:08:55 -0400
Subject: [PATCH] Issue #3414538: Enabled aggregation for our own JS if core
 patched:

See core issue #3232810 for explicit JS aggregation group patch.
---
 .../refreshless_turbo.libraries.yml           | 43 +++++++++++--------
 .../src/Hooks/Javascript.php                  | 34 +++++++++++++++
 2 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/modules/refreshless_turbo/refreshless_turbo.libraries.yml b/modules/refreshless_turbo/refreshless_turbo.libraries.yml
index 927ea7a..41b2542 100644
--- a/modules/refreshless_turbo/refreshless_turbo.libraries.yml
+++ b/modules/refreshless_turbo/refreshless_turbo.libraries.yml
@@ -3,25 +3,24 @@ refreshless:
     theme:
       css/turbo.css: {}
   js:
-    js/drupal_settings.js: { attributes: { defer: true }, preprocess: false }
-    js/behaviours.js: { attributes: { defer: true }, preprocess: false }
-    js/scroll.js: { attributes: { defer: true }, preprocess: false }
-    js/stylesheet_sorter.js: { attributes: { defer: true }, preprocess: false }
-    js/refreshless.js: {
-      attributes: {
-        defer: true,
-        # Causes Turbo to do a full page load if the query string for this
-        # changes, e.g. on a cache clear.
-        #
-        # @todo Should this be configurable?
-        data-turbo-track: 'reload',
-      },
-      # This should be excluded from aggregation for now.
-      #
-      # @see https://www.drupal.org/project/refreshless/issues/3414538
-      #   JavaScript aggregation issue.
-      preprocess: false,
-    }
+    # The explicit aggregation group should not be changed or removed as it's
+    # used to group our JavaScript into a separate aggregate if/when core
+    # supports it; if we detect that core supports this, the preprocess: false
+    # is changed to preprocess: true; if core does not support aggregation
+    # groups, the preprocess: false set here is left as-is to prevent our
+    # JavaScript potentially being downloaded and evaluated more than once since
+    # aggregation is not additive by default.
+    #
+    # @see https://www.drupal.org/project/drupal/issues/3232810
+    #   Core issue with a patch to enable setting explicit aggregation groups.
+    #
+    # @see https://www.drupal.org/project/refreshless/issues/3414538
+    #   RefreshLess issue implementing JavaScript aggregation support.
+    js/drupal_settings.js: { attributes: { defer: true }, group: refreshless-turbo, preprocess: false }
+    js/behaviours.js: { attributes: { defer: true }, group: refreshless-turbo, preprocess: false }
+    js/scroll.js: { attributes: { defer: true }, group: refreshless-turbo, preprocess: false }
+    js/stylesheet_sorter.js: { attributes: { defer: true }, group: refreshless-turbo, preprocess: false }
+    js/refreshless.js: { attributes: { defer: true }, group: refreshless-turbo, preprocess: false }
   header: true
   dependencies:
     - core/drupal
@@ -81,6 +80,12 @@ js-cookie:
 turbo:
   version: 8.0.10
   js:
+    # Note that at the time of writing, Turbo does not cope well with being
+    # aggregated (and/or minified?), so the lack of a group here is intentional
+    # to prevent our hook picking this up and setting preprocess: true if core
+    # supports setting explicit aggregation groups.
+    #
+    # @see https://www.drupal.org/project/refreshless/issues/3414538
     vendor/@hotwired/turbo/dist/turbo.es2017-umd.js: { attributes: { defer: true }, preprocess: false }
   header: true
   remote: https://turbo.hotwired.dev/
diff --git a/modules/refreshless_turbo/src/Hooks/Javascript.php b/modules/refreshless_turbo/src/Hooks/Javascript.php
index 9c980e8..2a257e5 100644
--- a/modules/refreshless_turbo/src/Hooks/Javascript.php
+++ b/modules/refreshless_turbo/src/Hooks/Javascript.php
@@ -76,6 +76,40 @@ class Javascript implements ContainerInjectionInterface {
 
   }
 
+  #[Alter('js')]
+  /**
+   * Enable aggregation for our JavaScript if core supports naming groups.
+   *
+   * If core does not support named aggregation groups, the 'group' value will
+   * never be our string but a numeric value that core sets; if it's the numeric
+   * value, we don't do anything and allow the preprocess: false in our library
+   * definition to be used as a fallback.
+   *
+   * Note that \hook_library_info_alter() seems to be too early to detect this
+   * as it'll always have our string group; core seems to override the group
+   * value sometime between invoking that hook and \hook_js_alter().
+   *
+   * @see \hook_js_alter()
+   */
+  public function refreshLessTurboGroupPreprocess(
+    array &$javascript,
+    AttachedAssetsInterface $assets, LanguageInterface $language,
+  ): void {
+
+    foreach ($javascript as $path => &$values) {
+
+      if ($values['group'] !== 'refreshless-turbo') {
+        continue;
+      }
+
+      // Named aggregation groups are supported, so enable aggregation for all
+      // files in our group.
+      $values['preprocess'] = true;
+
+    }
+
+  }
+
   #[Hook('js_settings_build')]
   public function outputPageState(
     array &$settings, AttachedAssetsInterface $assets,
-- 
GitLab