New API for minified files using injectable service

docs/en/02_Developer_Guides/01_Templates/03_Requirements.md

@@ -169,13 +169,13 @@ replaced. For instance, the below will set a new set of dependencies to write to
     ---
     Name: myrequirements
     ---
-    Requirements:
+    SilverStripe\View\Requirements:
       disable_flush_combined: true
-    Requirements_Backend:
+    SilverStripe\View\Requirements_Backend:
       combine_in_dev: true
       combine_hash_querystring: true
       default_combined_files_folder: 'combined'
-    Injector:
+    SilverStripe\Core\Injector\Injector:
       MySiteAdapter:
         class: 'SilverStripe\Filesystem\Flysystem\AssetAdapter'
         constructor:
@@ -192,7 +192,7 @@ replaced. For instance, the below will set a new set of dependencies to write to
         class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler
         properties:
           Filesystem: '%$MySiteBackend'
-      Requirements_Backend:
+      SilverStripe\View\Requirements_Backend:
         properties:
           AssetHandler: '%$MySiteAssetHandler'
 
@@ -248,6 +248,49 @@ $scripts = array(
 Requirements::combine_files('scripts.js', $scripts, array('async' => true, 'defer' => true));
 ```
 
+### Minification of CSS and JS files
+
+You can minify combined Javascript and CSS files at runtime using an implementation of the
+`SilverStripe\View\Requirements_Minifier` interface.
+
+```php
+namespace MyProject;
+
+use SilverStripe\View\Requirements_Minifier;
+
+class MyMinifier implements Requirements_Minifier
+{
+    /**
+     * Minify the given content
+     *
+     * @param string $content
+     * @param string $type Either js or css
+     * @param string $filename Name of file to display in case of error
+     * @return string minified content
+     */	
+	public function minify ($content, $type, $fileName)
+	{
+		// Minify $content;
+
+		return $minifiedContent;
+	}
+}
+```
+
+Then, inject this service in `Requirements_Backend`.
+
+```yaml
+SilverStripe\Core\Injector\Injector:
+  SilverStripe\View\Requirements_Backend:
+    properties:
+      Minifier: %$MyProject\MyMinifier
+```
+
+<div class="alert" markdown='1'>
+While the framework does afford you the option of minification at runtime, we recommend using one of many frontend build
+tools to do this for you, e.g. [Webpack](https://webpack.github.io/), [Gulp](http://gulpjs.com/), or [Grunt](https://gruntjs.com/).
+</div>
+
 
 ## Clearing assets
 

src/View/Requirements_Backend.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\View;
 
 use InvalidArgumentException;
+use Exception;
 use SilverStripe\Assets\File;
 use SilverStripe\Assets\Storage\GeneratedAssetHandler;
 use SilverStripe\Control\Director;
@@ -10,7 +11,6 @@ use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injectable;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\Debug;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\SapphireTest;
@@ -121,11 +121,11 @@ class Requirements_Backend
     protected $combinedFiles = array();
 
     /**
-     * Use the JSMin library to minify any javascript file passed to {@link combine_files()}.
+     * Use the injected minification service to minify any javascript file passed to {@link combine_files()}.
      *
      * @var bool
      */
-    protected $minifyCombinedJSFiles = true;
+    protected $minifyCombinedFiles = false;
 
     /**
      * Whether or not file headers should be written when combining files
@@ -191,6 +191,11 @@ class Requirements_Backend
      */
     protected $assetHandler = null;
 
+    /**
+     * @var Requirements_Minifier
+     */
+    protected $minifier = null;
+
     /**
      * Gets the backend storage for generated files
      *
@@ -211,6 +216,26 @@ class Requirements_Backend
         $this->assetHandler = $handler;
     }
 
+    /**
+     * Gets the minification service for this backend
+     *
+     * @return Requirements_Minifier
+     */
+    public function getMinifier()
+    {
+        return $this->minifier;
+    }
+
+    /**
+     * Set a new minification service for this backend
+     *
+     * @param Requirements_Minifier $minifier
+     */
+    public function setMinifier(Requirements_Minifier $minifier = null)
+    {
+        $this->minifier = $minifier;
+    }
+
     /**
      * Enable or disable the combination of CSS and JavaScript files
      *
@@ -340,24 +365,24 @@ class Requirements_Backend
     }
 
     /**
-     * Check if minify js files should be combined
+     * Check if minify files should be combined
      *
      * @return bool
      */
-    public function getMinifyCombinedJSFiles()
+    public function getMinifyCombinedFiles()
     {
-        return $this->minifyCombinedJSFiles;
+        return $this->minifyCombinedFiles;
     }
 
     /**
-     * Set if combined js files should be minified
+     * Set if combined files should be minified
      *
      * @param bool $minify
      * @return $this
      */
-    public function setMinifyCombinedJSFiles($minify)
+    public function setMinifyCombinedFiles($minify)
     {
-        $this->minifyCombinedJSFiles = $minify;
+        $this->minifyCombinedFiles = $minify;
         return $this;
     }
 
@@ -1278,7 +1303,20 @@ class Requirements_Backend
         $combinedFileID = File::join_paths($this->getCombinedFilesFolder(), $combinedFile);
 
         // Send file combination request to the backend, with an optional callback to perform regeneration
-        $minify = $this->getMinifyCombinedJSFiles();
+        $minify = $this->getMinifyCombinedFiles();
+        if ($minify && !$this->minifier) {
+            throw new Exception(
+                sprintf(
+                    'Cannot minify files without a minification service defined.
+        			Set %s::minifyCombinedFiles to false, or inject a %s service on 
+        			%s.properties.minifier',
+                    __CLASS__,
+                    Requirements_Minifier::class,
+                    __CLASS__
+                )
+            );
+        }
+
         $combinedURL = $this
             ->getAssetHandler()
             ->getContentURL(
@@ -1287,12 +1325,11 @@ class Requirements_Backend
                     // Physically combine all file content
                     $combinedData = '';
                     $base = Director::baseFolder() . '/';
-                    $minifier = Injector::inst()->get('SilverStripe\\View\\Requirements_Minifier');
                     foreach ($fileList as $file) {
                         $fileContent = file_get_contents($base . $file);
                         // Use configured minifier
                         if ($minify) {
-                            $fileContent = $minifier->minify($fileContent, $type, $file);
+                            $fileContent = $this->minifier->minify($fileContent, $type, $file);
                         }
 
                         if ($this->writeHeaderComment) {

tests/php/View/RequirementsTest.php

@@ -86,7 +86,7 @@ class RequirementsTest extends SapphireTest
         $backend->clear();
         $backend->clearCombinedFiles();
         $backend->setCombinedFilesFolder('_combinedfiles');
-        $backend->setMinifyCombinedJSFiles(false);
+        $backend->setMinifyCombinedFiles(false);
         Requirements::flush();
     }
 

tests/php/View/SSViewerTest.php

@@ -20,6 +20,7 @@ use SilverStripe\Security\SecurityToken;
 use SilverStripe\Security\Permission;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\Requirements_Backend;
+use SilverStripe\View\Requirements_Minifier;
 use SilverStripe\View\SSViewer;
 use SilverStripe\View\Requirements;
 use SilverStripe\View\Tests\SSViewerTest\SSViewerTestModel;
@@ -28,7 +29,6 @@ use SilverStripe\View\ViewableData;
 use SilverStripe\View\SSViewer_FromString;
 use SilverStripe\View\SSTemplateParser;
 use SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore;
-use JSMin;
 use Exception;
 
 classSSViewerTest  extends SapphireTest
@@ -230,15 +230,6 @@ class SSViewerTest extends SapphireTest
     $jsFileContents = file_get_contents(BASE_PATH . '/' . $jsFile);
     $testBackend->combineFiles('testRequirementsCombine.js', array($jsFile));
 
-        // first make sure that our test js file causes an exception to be thrown
-        try {
-            include_once 'jsmin/jsmin.php';
-            JSMin::minify($jsFileContents);
-            $this->fail('JSMin did not throw exception on minify bad file: ');
-        } catch (Exception $e) {
-            // exception thrown... good
-        }
-
     // secondly, make sure that requirements is generated, even though minification failed
     $testBackend->processCombinedFiles();
     $js = array_keys($testBackend->getJavascript());
@@ -253,6 +244,44 @@ class SSViewerTest extends SapphireTest
     $this->assertContains($jsFileContents, $combinedTestFileContents);
 }
 
+public function testRequirementsMinification()
+{
+    $testBackend = Injector::inst()->create(Requirements_Backend::class);
+    $testBackend->setSuffixRequirements(false);
+    $testBackend->setMinifyCombinedFiles(true);
+    $testFile = $this->getCurrentRelativePath() . '/SSViewerTest/javascript/RequirementsTest_a.js';
+    $testFileContent = file_get_contents($testFile);
+
+    $mockMinifier = $this->getMockBuilder(Requirements_Minifier::class)
+        ->setMethods(['minify'])
+        ->getMock();
+
+    $mockMinifier->expects($this->once())
+        ->method('minify')
+        ->with(
+            $testFileContent,
+            'js',
+            $testFile
+        );
+    $testBackend->setMinifier($mockMinifier);
+    $testBackend->combineFiles('testRequirementsMinified.js', array($testFile));
+    $testBackend->processCombinedFiles();
+
+    $testBackend->setMinifyCombinedFiles(false);
+    $mockMinifier->expects($this->never())
+        ->method('minify');
+    $testBackend->processCombinedFiles();
+
+    $this->setExpectedExceptionRegExp(
+        Exception::class,
+        '/minification service/'
+    );
+
+    $testBackend->setMinifyCombinedFiles(true);
+    $testBackend->setMinifier(null);
+    $testBackend->processCombinedFiles();
+}
+
 
 
 public function testComments()