Fix CRM_Utils_JS::dedupeClosures to ignore comments

This runs stripComments BEFORE dedupeClosures so that comments don't interfere with the deduping. Also fixes stripComments to remove the line break as well as the comment line.
civicrm · Jun 30, 2020 · 0290b3e · 0290b3e
1 parent 06c114d
commit 0290b3e
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 9 deletions.
diff --git a/CRM/Utils/JS.php b/CRM/Utils/JS.php
@@ -109,7 +109,7 @@ public static function dedupeClosures($scripts, $localVars, $inputVals) {
    * @return string
    */
   public static function stripComments($script) {
-    return preg_replace(":^\\s*//[^\n]+$:m", "", $script);
+    return preg_replace("#^\\s*//[^\n]*$(?:\r\n|\n)?#m", "", $script);
   }
 
   /**

diff --git a/Civi/Angular/Page/Modules.php b/Civi/Angular/Page/Modules.php
@@ -106,15 +106,14 @@ public static function buildAngularModules($event) {
   public function digestJs($files) {
     $scripts = [];
     foreach ($files as $file) {
-      $scripts[] = file_get_contents($file);
+      $scripts[] = \CRM_Utils_JS::stripComments(file_get_contents($file));
     }
     $scripts = \CRM_Utils_JS::dedupeClosures(
       $scripts,
       ['angular', '$', '_'],
       ['angular', 'CRM.$', 'CRM._']
     );
-    // This impl of stripComments currently adds 10-20ms and cuts ~7%
-    return \CRM_Utils_JS::stripComments(implode("\n", $scripts));
+    return implode("\n", $scripts);
   }
 
   /**

diff --git a/ang/crmUi.js b/ang/crmUi.js
@@ -484,7 +484,6 @@
     //         $scope.myOrder.setDir('field2', '');
     //   HTML: <tr ng-repeat="... | order:myOrder.get()">...</tr>
     .service('CrmUiOrderCtrl', function(){
-      //
       function CrmUiOrderCtrl(defaults){
         this.values = defaults;
       }

diff --git a/tests/phpunit/CRM/Utils/JSTest.php b/tests/phpunit/CRM/Utils/JSTest.php
@@ -155,23 +155,27 @@ public function stripCommentsExamples() {
     $cases = [];
     $cases[] = [
       "a();\n//# sourceMappingURL=../foo/bar/baz.js\nb();",
-      "a();\n\nb();",
+      "a();\nb();",
     ];
     $cases[] = [
       "// foo\na();",
-      "\na();",
+      "a();",
+    ];
+    $cases[] = [
+      "// foo\n //\na();",
+      "a();",
     ];
     $cases[] = [
       "b();\n  // foo",
       "b();\n",
     ];
     $cases[] = [
       "/// foo\na();\n\t \t//bar\nb();\n// whiz",
-      "\na();\n\nb();\n",
+      "a();\nb();\n",
     ];
     $cases[] = [
       "alert('//# sourceMappingURL=../foo/bar/baz.js');\n//zoop\na();",
-      "alert('//# sourceMappingURL=../foo/bar/baz.js');\n\na();",
+      "alert('//# sourceMappingURL=../foo/bar/baz.js');\na();",
     ];
     return $cases;
   }