Add asset priority & use AssetMaker for global backend assets (#1022)

Co-authored-by: Luke Towers <luke@luketowers.ca>
Co-authored-by: Luke Towers <git@luketowers.ca>

This PR changes the global backend assets that were originally specified directly in the "head" template for the Backend, and uses the AssetMaker to load them instead. This should allow people the ability to manipulate the assets if need be through events.

To ensure that the global assets are loaded first before all others, I have also introduced a order value for assets. It may be specified by providing the second argument for addJs(), addCss(), etc. as an array and populating it with the following:

$this->addCss('my.css', [
    'build' => 'core',
    'order' => 50, // must be an integer
]);

The default order for all assets is 500.

modules/backend/classes/Controller.php

@@ -1,13 +1,13 @@
-<?php namespace Backend\Classes;
+<?php
+
+namespace Backend\Classes;
 
 use Lang;
 use View;
 use Flash;
 use Config;
-use Closure;
 use Request;
 use Backend;
-use Session;
 use Redirect;
 use Response;
 use Exception;

modules/backend/layouts/_head.php

@@ -12,14 +12,18 @@
     <?= e(trans($this->pageTitle)) ?> | <?= e(Backend\Models\BrandSetting::get('app_name')) ?>
 </title>
 <?php
-$coreBuild = System\Models\Parameter::get('system::core.build', 1);
-
 // Styles
 $styles = [
     Url::asset('modules/system/assets/ui/storm.css'),
     Url::asset('modules/system/assets/ui/icons.css'),
     Backend::skinAsset('assets/css/winter.css'),
 ];
+foreach ($styles as $style) {
+    $this->addCss($style, [
+        'build' => 'core',
+        'order' => 1,
+    ]);
+}
 
 // Scripts
 $scripts = [
@@ -49,18 +53,14 @@
     Backend::skinAsset('assets/js/winter.flyout.js'),
     Backend::skinAsset('assets/js/winter.tabformexpandcontrols.js'),
 ]);
+foreach ($scripts as $script) {
+    $this->addJs($script, [
+        'build' => 'core',
+        'order' => 1,
+    ]);
+}
 ?>
 
-<?php foreach ($styles as $style): ?>
-    <link href="<?= $style . '?v=' . $coreBuild; ?>" rel="stylesheet" importance="high">
-    <link href="<?= $style . '?v=' . $coreBuild; ?>" rel="preload" as="style" importance="high">
-<?php endforeach; ?>
-
-<?php foreach ($scripts as $script): ?>
-    <script data-cfasync="false" src="<?= $script . '?v=' . $coreBuild; ?>" importance="high"></script>
-    <link href="<?= $script . '?v=' . $coreBuild; ?>" rel="preload" as="script" importance="high">
-<?php endforeach; ?>
-
 <?php if (!Config::get('cms.enableBackendServiceWorkers', false)): ?>
     <script>
         "use strict";

modules/backend/layouts/auth.php

@@ -17,6 +17,12 @@
             Url::asset('modules/system/assets/ui/icons.css'),
             Backend::skinAsset('assets/css/winter.css'),
         ];
+        foreach ($styles as $style) {
+            $this->addCss($style, [
+                'build' => 'core',
+                'order' => 1,
+            ]);
+        }
         $scripts = [
             Backend::skinAsset('assets/js/vendor/jquery.min.js'),
             Backend::skinAsset('assets/js/vendor/jquery-migrate.min.js'),
@@ -26,18 +32,14 @@
             Url::asset('modules/backend/assets/js/auth/auth.js'),
             Url::asset('modules/system/assets/js/lang/lang.'.App::getLocale().'.js'),
         ];
+        foreach ($scripts as $script) {
+            $this->addJs($script, [
+                'build' => 'core',
+                'order' => 1,
+            ]);
+        }
         ?>
 
-        <?php foreach ($styles as $style): ?>
-            <link href="<?= $style . '?v=' . $coreBuild; ?>" rel="stylesheet" importance="high">
-            <link href="<?= $style . '?v=' . $coreBuild; ?>" rel="preload" as="style" importance="high">
-        <?php endforeach; ?>
-
-        <?php foreach ($scripts as $script): ?>
-            <script data-cfasync="false" src="<?= $script . '?v=' . $coreBuild; ?>" importance="high"></script>
-            <link href="<?= $script . '?v=' . $coreBuild; ?>" rel="preload" as="script" importance="high">
-        <?php endforeach; ?>
-
         <?php if (!Config::get('cms.enableBackendServiceWorkers', false)): ?>
             <script>
                 "use strict";

modules/system/tests/traits/AssetMakerTest.php

@@ -4,17 +4,20 @@
 
 use System\Tests\Bootstrap\TestCase;
 use System\Traits\AssetMaker;
+use System\Traits\EventEmitter;
 use System\Traits\ViewMaker;
+use Winter\Storm\Support\Facades\Url;
 
 class AssetMakerStub
 {
     use AssetMaker;
-    use ViewMaker; // Needed for guessViewPath(), which is used to set default assetPath
+    use ViewMaker; // Needed for `guessViewPath()`, which is used to set default assetPath
+    use EventEmitter; // Needed for `addAsset()`
 }
 
 class AssetMakerTest extends TestCase
 {
-    private $stub;
+    private AssetMakerStub $stub;
 
     public function setUp() : void
     {
@@ -26,7 +29,7 @@ public function setUp() : void
     // Tests
     //
 
-    public function testGetLocalPath()
+    public function testGetLocalPath(): void
     {
         $basePath = base_path();
 
@@ -50,28 +53,69 @@ public function testGetLocalPath()
         $this->assertEquals(realpath($basePath.'/some/wild/absolute/path/'), realpath($resolvedPath));
     }
 
-    public function testGetAssetPath()
+    public function testGetAssetPath(): void
     {
         $assetPath = 'my/path/assets';
 
+        $hostUrl = Url::to('/');
+
         // assetPath is ignored since we use pathSymbol for plugins
         $path = $this->stub->getAssetPath('$/author/plugin/assets/js/myAsset.js', $assetPath);
-        $this->assertEquals('http://localhost/plugins/author/plugin/assets/js/myAsset.js', $path);
+        $this->assertEquals($hostUrl . 'plugins/author/plugin/assets/js/myAsset.js', $path);
 
         // assetPath is ignored since we use pathSymbol for theme
         $path = $this->stub->getAssetPath('#/mytheme/assets/js/myAsset.js', $assetPath);
-        $this->assertEquals('http://localhost/themes/mytheme/assets/js/myAsset.js', $path);
+        $this->assertEquals($hostUrl . 'themes/mytheme/assets/js/myAsset.js', $path);
 
         // assetPath is ignored since we use pathSymbol for app root
         $path = $this->stub->getAssetPath('~/plugins/author/plugin/assets/js/myAsset.js', $assetPath);
-        $this->assertEquals('http://localhost/plugins/author/plugin/assets/js/myAsset.js', $path);
+        $this->assertEquals($hostUrl . 'plugins/author/plugin/assets/js/myAsset.js', $path);
 
         // assetPath is used since we use a relative path without pathSymbol
         $path = $this->stub->getAssetPath('js/myAsset.js', $assetPath);
-        $this->assertEquals('http://localhost/' . $assetPath . '/js/myAsset.js', $path);
+        $this->assertEquals($hostUrl . $assetPath . '/js/myAsset.js', $path);
 
         // assetPath is ignored since we use an absolute path
         $path = $this->stub->getAssetPath('/js/myAsset.js', $assetPath);
-        $this->assertEquals('http://localhost/js/myAsset.js', $path);
+        $this->assertEquals($hostUrl . 'js/myAsset.js', $path);
+    }
+
+    public function testAssetOrdering(): void
+    {
+        $hostUrl = Url::to('/');
+
+        // Test specified priorities
+        $this->stub->addCss('mySecond.css', [
+            'order' => 2,
+        ]);
+        $this->stub->addCss('myThird.css', [
+            'order' => 3,
+        ]);
+        $this->stub->addCss('myFirst.css', [
+            'order' => 1,
+        ]);
+
+        $assets = $this->stub->getAssetPaths();
+
+        $this->assertEquals([
+            $hostUrl . 'myFirst.css',
+            $hostUrl . 'mySecond.css',
+            $hostUrl . 'myThird.css',
+        ], $assets['css']);
+
+        // Test first-come, first-served - these assets will be prioritised the default 100.
+        $this->stub->flushAssets();
+
+        $this->stub->addCss('myFirst.css');
+        $this->stub->addCss('mySecond.css');
+        $this->stub->addCss('myThird.css');
+
+        $assets = $this->stub->getAssetPaths();
+
+        $this->assertEquals([
+            $hostUrl . 'myFirst.css',
+            $hostUrl . 'mySecond.css',
+            $hostUrl . 'myThird.css',
+        ], $assets['css']);
     }
 }