From 8e1ae55ff67647c495c4e1d528907d280d78ad37 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 14 Jan 2016 17:23:21 +1300 Subject: [PATCH] API Enable single javascript files to declare that they include other files --- .../01_Templates/03_Requirements.md | 15 ++ docs/en/04_Changelogs/4.0.0.md | 16 ++ tests/forms/RequirementsTest.php | 64 +++++ view/Requirements.php | 247 ++++++++++++------ 4 files changed, 255 insertions(+), 87 deletions(-) diff --git a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md index 4f1226433..c3beed9a6 100644 --- a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md +++ b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md @@ -66,6 +66,21 @@ JavaScript in a separate file and instead load, via search and replace, several In this example, `editor.template.js` is expected to contain a replaceable variable expressed as `$EditorCSS`. +If you are using front-end script combination mechanisms, you can optionally declare +that your included files provide these scripts. This will ensure that subsequent +Requirement calls that rely on those included scripts will not double include those +files. + + + :::php + Requirements::javascript('mysite/js/dist/bundle.js', ['provides' => [ + 'mysite/js/jquery.js' + 'mysite/js/src/main.js', + 'mysite/js/src/functions.js' + ]]); + Requirements::javascript('mysite/js/jquery.js'); // Will will skip this file + + ### Custom Inline CSS or Javascript You can also quote custom script directly. This may seem a bit ugly, but is useful when you need to transfer some kind diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 22a005809..5ed6215a4 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -137,6 +137,22 @@ And some methods on `Requirements` and `Requirements_Backend` have been removed A new config `Requirements_Backend.combine_in_dev` has been added in order to allow combined files to be forced on during development. If this is off, combined files is only enabled in live environments. +In addition, a new API for javascript files has been added to support front-end tools +such as [browserify](http://browserify.org/) that pre-combine scripts. You can specify +a 'provides' option to the second parameter of `Requirements::javascript` to declare +included files. + +E.g. + + + :::php + Requirements::javascript('mysite/js/dist/bundle.js', ['provides' => [ + 'mysite/js/jquery.js' + 'mysite/js/src/main.js', + 'mysite/js/src/functions.js' + ]]); + + ## Upgrading ### Update code that uses SQLQuery diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index 29b1d4270..34b166c51 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -22,6 +22,7 @@ class RequirementsTest extends SapphireTest { } public function testExternalUrls() { + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $backend->setCombinedFilesEnabled(true); @@ -118,6 +119,7 @@ class RequirementsTest extends SapphireTest { } public function testCombinedJavascript() { + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedRequirements($backend); @@ -166,6 +168,7 @@ class RequirementsTest extends SapphireTest { // Then do it again, this time not requiring the files beforehand unlink($combinedFilePath); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedNonrequiredRequirements($backend); $html = $backend->includeInHTML(false, self::$html_template); @@ -204,6 +207,7 @@ class RequirementsTest extends SapphireTest { public function testCombinedCss() { $basePath = $this->getCurrentRelativePath(); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); @@ -230,6 +234,7 @@ class RequirementsTest extends SapphireTest { ); // Test that combining a file multiple times doesn't trigger an error + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->combineFiles( @@ -257,6 +262,7 @@ class RequirementsTest extends SapphireTest { public function testBlockedCombinedJavascript() { $basePath = $this->getCurrentRelativePath(); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedRequirements($backend); $combinedFileName = '/_combinedfiles/RequirementsTest_bc-51622b5.js'; @@ -314,6 +320,7 @@ class RequirementsTest extends SapphireTest { public function testArgsInUrls() { $basePath = $this->getCurrentRelativePath(); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); @@ -339,6 +346,7 @@ class RequirementsTest extends SapphireTest { public function testRequirementsBackend() { $basePath = $this->getCurrentRelativePath(); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($basePath . '/a.js'); @@ -377,6 +385,7 @@ class RequirementsTest extends SapphireTest { // to something else $basePath = 'framework' . substr($basePath, strlen(FRAMEWORK_DIR)); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $holder = Requirements::backend(); @@ -406,6 +415,7 @@ class RequirementsTest extends SapphireTest { } public function testJsWriteToBody() { + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript('http://www.mydomain.com/test.js'); @@ -425,6 +435,7 @@ class RequirementsTest extends SapphireTest { public function testIncludedJsIsNotCommentedOut() { $template = ''; + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($this->getCurrentRelativePath() . '/RequirementsTest_a.js'); @@ -437,6 +448,7 @@ class RequirementsTest extends SapphireTest { public function testCommentedOutScriptTagIsIgnored() { $template = '' . '

more content

'; + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->setSuffixRequirements(false); @@ -449,6 +461,7 @@ class RequirementsTest extends SapphireTest { } public function testForceJsToBottom() { + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript('http://www.mydomain.com/test.js'); @@ -505,6 +518,7 @@ class RequirementsTest extends SapphireTest { $template = '
My header

Body

'; $basePath = $this->getCurrentRelativePath(); + /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); @@ -529,6 +543,56 @@ class RequirementsTest extends SapphireTest { $this->assertNotRegexp('/RequirementsTest_b\.css\?m=[\d]*&foo=bar&bla=blubb"/', $html); } + /** + * Tests that provided files work + */ + public function testProvidedFiles() { + /** @var Requirements_Backend $backend */ + $template = '
My header

Body

'; + $basePath = $this->getCurrentRelativePath(); + + // Test that provided files block subsequent files + $backend = Injector::inst()->create('Requirements_Backend'); + $this->setupRequirements($backend); + $backend->javascript($basePath . '/RequirementsTest_a.js'); + $backend->javascript($basePath . '/RequirementsTest_b.js', [ + 'provides' => [ + $basePath . '/RequirementsTest_a.js', + $basePath . '/RequirementsTest_c.js' + ] + ]); + $backend->javascript($basePath . '/RequirementsTest_c.js'); + // Note that _a.js isn't considered provided because it was included + // before it was marked as provided + $this->assertEquals([ + $basePath . '/RequirementsTest_c.js' => $basePath . '/RequirementsTest_c.js' + ], $backend->getProvidedScripts()); + $html = $backend->includeInHTML(false, $template); + $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html); + $this->assertRegExp('/src=".*\/RequirementsTest_b\.js/', $html); + $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html); + + // Test that provided files block subsequent combined files + $backend = Injector::inst()->create('Requirements_Backend'); + $this->setupRequirements($backend); + $backend->combineFiles('combined_a.js', [$basePath . '/RequirementsTest_a.js']); + $backend->javascript($basePath . '/RequirementsTest_b.js', [ + 'provides' => [ + $basePath . '/RequirementsTest_a.js', + $basePath . '/RequirementsTest_c.js' + ] + ]); + $backend->combineFiles('combined_c.js', [$basePath . '/RequirementsTest_c.js']); + $this->assertEquals([ + $basePath . '/RequirementsTest_c.js' => $basePath . '/RequirementsTest_c.js' + ], $backend->getProvidedScripts()); + $html = $backend->includeInHTML(false, $template); + $this->assertRegExp('/src=".*\/combined_a/', $html); + $this->assertRegExp('/src=".*\/RequirementsTest_b\.js/', $html); + $this->assertNotRegExp('/src=".*\/combined_c/', $html); + $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html); + } + /** * Verify that the given backend includes the given files * diff --git a/view/Requirements.php b/view/Requirements.php index 745bbbc0c..3c1a25443 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -110,9 +110,11 @@ class Requirements implements Flushable { * Register the given JavaScript file as required. * * @param string $file Relative to docroot + * @param array $options List of options. Available options include: + * - 'provides' : List of scripts files included in this file */ - public static function javascript($file) { - self::backend()->javascript($file); + public static function javascript($file, $options = array()) { + self::backend()->javascript($file, $options); } /** @@ -497,6 +499,14 @@ class Requirements_Backend */ protected $javascript = array(); + /** + * Map of included scripts to array of contained files. + * To be used alongside front-end combination mechanisms. + * + * @var array Map of providing filepath => array(provided filepaths) + */ + protected $providedJavascript = array(); + /** * Paths to all required CSS files relative to the docroot. * @@ -784,9 +794,16 @@ class Requirements_Backend * Register the given JavaScript file as required. * * @param string $file Relative to docroot + * @param array $options List of options. Available options include: + * - 'provides' : List of scripts files included in this file */ - public function javascript($file) { + public function javascript($file, $options = array()) { $this->javascript[$file] = true; + + // Record scripts included in this file + if(isset($options['provides'])) { + $this->providedJavascript[$file] = array_values($options['provides']); + } } /** @@ -798,13 +815,58 @@ class Requirements_Backend unset($this->javascript[$file]); } + /** + * Gets all scripts that are already provided by prior scripts. + * This follows these rules: + * - Files will not be considered provided if they are separately + * included prior to the providing file. + * - Providing files can be blocked, and don't provide anything + * - Provided files can't be blocked (you need to block the provider) + * - If a combined file includes files that are provided by prior + * scripts, then these should be excluded from the combined file. + * - If a combined file includes files that are provided by later + * scripts, then these files should be included in the combined + * file, but we can't block the later script either (possible double + * up of file). + * + * @return array Array of provided files (map of $path => $path) + */ + public function getProvidedScripts() { + $providedScripts = array(); + $includedScripts = array(); + foreach($this->javascript as $script => $flag) { + // Ignore scripts that are explicitly blocked + if(isset($this->blocked[$script])) { + continue; + } + // At this point, the file is included. + // This might also be combined at this point, potentially. + $includedScripts[$script] = true; + + // Record any files this provides, EXCEPT those already included by now + if(isset($this->providedJavascript[$script])) { + foreach($this->providedJavascript[$script] as $provided) { + if(!isset($includedScripts[$provided])) { + $providedScripts[$provided] = $provided; + } + } + } + } + return $providedScripts; + } + /** * Returns an array of required JavaScript, excluding blocked + * and duplicates of provided files. * * @return array */ public function getJavascript() { - return array_keys(array_diff_key($this->javascript, $this->blocked)); + return array_keys(array_diff_key( + $this->javascript, + $this->getBlocked(), + $this->getProvidedScripts() + )); } /** @@ -1044,90 +1106,90 @@ class Requirements_Backend * @return string HTML content augmented with the requirements tags */ public function includeInHTML($templateFile, $content) { - if( - (strpos($content, '') !== false || strpos($content, 'css || $this->javascript || $this->customCSS || $this->customScript || $this->customHeadTags) - ) { - $requirements = ''; - $jsRequirements = ''; + // Skip if content isn't injectable, or there is nothing to inject + $tagsAvailable = preg_match('#css || $this->javascript || $this->customCSS || $this->customScript || $this->customHeadTags; + if(!$tagsAvailable || !$hasFiles) { + return $content; + } + $requirements = ''; + $jsRequirements = ''; - // Combine files - updates $this->javascript and $this->css - $this->processCombinedFiles(); + // Combine files - updates $this->javascript and $this->css + $this->processCombinedFiles(); - foreach($this->getJavascript() as $file) { - $path = Convert::raw2xml($this->pathForFile($file)); - if($path) { - $jsRequirements .= "\n"; - } - } - - // Add all inline JavaScript *after* including external files they might rely on - foreach($this->getCustomScripts() as $script) { - $jsRequirements .= "\n"; - } - - foreach($this->getCSS() as $file => $params) { - $path = Convert::raw2xml($this->pathForFile($file)); - if($path) { - $media = (isset($params['media']) && !empty($params['media'])) - ? " media=\"{$params['media']}\"" : ""; - $requirements .= "\n"; - } - } - - foreach($this->getCustomCSS() as $css) { - $requirements .= "\n"; - } - - foreach($this->getCustomHeadTags() as $customHeadTag) { - $requirements .= "$customHeadTag\n"; - } - - if ($this->getForceJSToBottom()) { - // Remove all newlines from code to preserve layout - $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); - - // Forcefully put the scripts at the bottom of the body instead of before the first - // script tag. - $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); - - // Put CSS at the bottom of the head - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); - } elseif($this->getWriteJavascriptToBody()) { - // Remove all newlines from code to preserve layout - $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); - - // If your template already has script tags in the body, then we try to put our script - // tags just before those. Otherwise, we put it at the bottom. - $p2 = stripos($content, '))/U', $content, $commentTags, 0, $p1) - && - $commentTags[1] == '-->' - ); - - if($canWriteToBody) { - $content = substr($content,0,$p1) . $jsRequirements . substr($content,$p1); - } else { - $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); - } - - // Put CSS at the bottom of the head - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); - } else { - $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); - $content = preg_replace("/(<\/head>)/i", $jsRequirements . "\\1", $content); + foreach($this->getJavascript() as $file) { + $path = Convert::raw2xml($this->pathForFile($file)); + if($path) { + $jsRequirements .= "\n"; } } + // Add all inline JavaScript *after* including external files they might rely on + foreach($this->getCustomScripts() as $script) { + $jsRequirements .= "\n"; + } + + foreach($this->getCSS() as $file => $params) { + $path = Convert::raw2xml($this->pathForFile($file)); + if($path) { + $media = (isset($params['media']) && !empty($params['media'])) + ? " media=\"{$params['media']}\"" : ""; + $requirements .= "\n"; + } + } + + foreach($this->getCustomCSS() as $css) { + $requirements .= "\n"; + } + + foreach($this->getCustomHeadTags() as $customHeadTag) { + $requirements .= "$customHeadTag\n"; + } + + if ($this->getForceJSToBottom()) { + // Remove all newlines from code to preserve layout + $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); + + // Forcefully put the scripts at the bottom of the body instead of before the first + // script tag. + $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); + + // Put CSS at the bottom of the head + $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); + } elseif($this->getWriteJavascriptToBody()) { + // Remove all newlines from code to preserve layout + $jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements); + + // If your template already has script tags in the body, then we try to put our script + // tags just before those. Otherwise, we put it at the bottom. + $p2 = stripos($content, '))/U', $content, $commentTags, 0, $p1) + && + $commentTags[1] == '-->' + ); + + if($canWriteToBody) { + $content = substr($content,0,$p1) . $jsRequirements . substr($content,$p1); + } else { + $content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content); + } + + // Put CSS at the bottom of the head + $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); + } else { + $content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content); + $content = preg_replace("/(<\/head>)/i", $jsRequirements . "\\1", $content); + } return $content; } @@ -1426,6 +1488,9 @@ class Requirements_Backend return; } + // Before scripts are modified, detect files that are provided by preceding ones + $providedScripts = $this->getProvidedScripts(); + // Process each combined files foreach($this->getAllCombinedFiles() as $combinedFile => $combinedItem) { $fileList = $combinedItem['files']; @@ -1435,7 +1500,13 @@ class Requirements_Backend // Generate this file, unless blocked $combinedURL = null; if(!isset($this->blocked[$combinedFile])) { - $combinedURL = $this->getCombinedFileURL($combinedFile, $fileList, $type); + // Filter files for blocked / provided + $filteredFileList = array_diff( + $fileList, + $this->getBlocked(), + $providedScripts + ); + $combinedURL = $this->getCombinedFileURL($combinedFile, $filteredFileList, $type); } // Replace all existing files, injecting the combined file at the position of the first item @@ -1483,11 +1554,13 @@ class Requirements_Backend * @param string $combinedFile Filename for this combined file * @param array $fileList List of files to combine * @param string $type Either 'js' or 'css' - * @return string URL to this resource + * @return string|null URL to this resource, if there are files to combine */ protected function getCombinedFileURL($combinedFile, $fileList, $type) { - // Filter blocked files - $fileList = array_diff($fileList, $this->getBlocked()); + // Skip empty lists + if(empty($fileList)) { + return null; + } // Generate path (Filename) $hashQuerystring = Config::inst()->get(static::class, 'combine_hash_querystring');