From abc1e0c2bb434e628c45accaf44fff378ad36d75 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 11 Oct 2013 01:54:05 +0200 Subject: [PATCH] fix issue #700 adjust depended asset position or throw exception on conflict --- framework/yii/base/View.php | 45 ++++++++-- framework/yii/web/AssetBundle.php | 5 -- tests/unit/data/views/layout.php | 22 +++++ tests/unit/data/views/rawlayout.php | 5 ++ tests/unit/data/views/simple.php | 1 + tests/unit/framework/web/AssetBundleTest.php | 126 +++++++++++++++++++++++++++ 6 files changed, 194 insertions(+), 10 deletions(-) create mode 100644 tests/unit/data/views/layout.php create mode 100644 tests/unit/data/views/rawlayout.php create mode 100644 tests/unit/data/views/simple.php diff --git a/framework/yii/base/View.php b/framework/yii/base/View.php index 9c6a339..1b656d2 100644 --- a/framework/yii/base/View.php +++ b/framework/yii/base/View.php @@ -146,7 +146,7 @@ class View extends Component * are the registered [[AssetBundle]] objects. * @see registerAssetBundle */ - public $assetBundles; + public $assetBundles = array(); /** * @var string the page title */ @@ -523,6 +523,9 @@ class View extends Component $this->trigger(self::EVENT_END_PAGE); $content = ob_get_clean(); + foreach(array_keys($this->assetBundles) as $bundle) { + $this->registerAssetFiles($bundle); + } echo strtr($content, array( self::PH_HEAD => $this->renderHeadHtml(), self::PH_BODY_BEGIN => $this->renderBodyBeginHtml(), @@ -530,7 +533,6 @@ class View extends Component )); unset( - $this->assetBundles, $this->metaTags, $this->linkTags, $this->css, @@ -566,25 +568,58 @@ class View extends Component echo self::PH_HEAD; } + protected function registerAssetFiles($name) + { + if (!isset($this->assetBundles[$name])) { + return; + } + $bundle = $this->assetBundles[$name]; + foreach($bundle->depends as $depName) { + $this->registerAssetFiles($depName); + } + $bundle->registerAssets($this); + unset($this->assetBundles[$name]); + } + /** * Registers the named asset bundle. * All dependent asset bundles will be registered. * @param string $name the name of the asset bundle. + * @param integer|null $position optional parameter to force a minimum Javascript position TODO link to relevant method + * Null means to register on default position. * @return AssetBundle the registered asset bundle instance * @throws InvalidConfigException if the asset bundle does not exist or a circular dependency is detected */ - public function registerAssetBundle($name) + public function registerAssetBundle($name, $position = null) { if (!isset($this->assetBundles[$name])) { $am = $this->getAssetManager(); $bundle = $am->getBundle($name); $this->assetBundles[$name] = false; - $bundle->registerAssets($this); + // register dependencies + $pos = isset($bundle->jsOptions['position']) ? $bundle->jsOptions['position'] : null; + foreach ($bundle->depends as $dep) { + $this->registerAssetBundle($dep, $pos); + } $this->assetBundles[$name] = $bundle; } elseif ($this->assetBundles[$name] === false) { throw new InvalidConfigException("A circular dependency is detected for bundle '$name'."); + } else { + $bundle = $this->assetBundles[$name]; + } + + if ($position !== null) { + $pos = isset($bundle->jsOptions['position']) ? $bundle->jsOptions['position'] : null; + if ($pos === null) { + $bundle->jsOptions['position'] = $pos = $position; + } elseif ($pos > $position) { + throw new InvalidConfigException("A dependend AssetBundle of '$name' requires a higher position but it conflicts with the position set for '$name'!"); + } + foreach ($bundle->depends as $dep) { + $this->registerAssetBundle($dep, $pos); + } } - return $this->assetBundles[$name]; + return $bundle; } /** diff --git a/framework/yii/web/AssetBundle.php b/framework/yii/web/AssetBundle.php index d324ef3..aa2d02b 100644 --- a/framework/yii/web/AssetBundle.php +++ b/framework/yii/web/AssetBundle.php @@ -130,7 +130,6 @@ class AssetBundle extends Object /** * Registers the CSS and JS files with the given view. - * This method will first register all dependent asset bundles. * It will then try to convert non-CSS or JS files (e.g. LESS, Sass) into the corresponding * CSS or JS files using [[AssetManager::converter|asset converter]]. * @param \yii\base\View $view the view that the asset files to be registered with. @@ -139,10 +138,6 @@ class AssetBundle extends Object */ public function registerAssets($view) { - foreach ($this->depends as $name) { - $view->registerAssetBundle($name); - } - $this->publish($view->getAssetManager()); foreach ($this->js as $js) { diff --git a/tests/unit/data/views/layout.php b/tests/unit/data/views/layout.php new file mode 100644 index 0000000..cd9d9d6 --- /dev/null +++ b/tests/unit/data/views/layout.php @@ -0,0 +1,22 @@ + +beginPage(); ?> + + + + Test + head(); ?> + + +beginBody(); ?> + + + +endBody(); ?> + + +endPage(); ?> \ No newline at end of file diff --git a/tests/unit/data/views/rawlayout.php b/tests/unit/data/views/rawlayout.php new file mode 100644 index 0000000..6864642 --- /dev/null +++ b/tests/unit/data/views/rawlayout.php @@ -0,0 +1,5 @@ +beginPage(); ?>1head(); ?>2beginBody(); ?>3endBody(); ?>4endPage(); ?> \ No newline at end of file diff --git a/tests/unit/data/views/simple.php b/tests/unit/data/views/simple.php new file mode 100644 index 0000000..437ba90 --- /dev/null +++ b/tests/unit/data/views/simple.php @@ -0,0 +1 @@ +This is a damn simple view file. \ No newline at end of file diff --git a/tests/unit/framework/web/AssetBundleTest.php b/tests/unit/framework/web/AssetBundleTest.php index 60bb7eb..02cb5ed 100644 --- a/tests/unit/framework/web/AssetBundleTest.php +++ b/tests/unit/framework/web/AssetBundleTest.php @@ -45,6 +45,13 @@ class AssetBundleTest extends \yiiunit\TestCase TestJqueryAsset::register($view); $this->assertEquals(1, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $expected = << +4 +EOF; + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); } public function testSimpleDependency() @@ -56,6 +63,125 @@ class AssetBundleTest extends \yiiunit\TestCase $this->assertEquals(2, count($view->assetBundles)); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $expected = << +23 + +4 +EOF; + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); + } + + public function positionProvider() + { + return array( + array(View::POS_HEAD, true), + array(View::POS_HEAD, false), + array(View::POS_BEGIN, true), + array(View::POS_BEGIN, false), + array(View::POS_END, true), + array(View::POS_END, false), + ); + } + + /** + * @dataProvider positionProvider + */ + public function testPositionDependency($pos, $jqAlreadyRegistered) + { + $view = $this->getView(); + + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestAssetBundle'] = array( + 'jsOptions' => array( + 'position' => $pos, + ), + ); + + $this->assertEmpty($view->assetBundles); + if ($jqAlreadyRegistered) { + TestJqueryAsset::register($view); + } + TestAssetBundle::register($view); + $this->assertEquals(2, count($view->assetBundles)); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestAssetBundle', $view->assetBundles); + $this->assertArrayHasKey('yiiunit\\framework\\web\\TestJqueryAsset', $view->assetBundles); + + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle'] instanceof AssetBundle); + $this->assertTrue($view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset'] instanceof AssetBundle); + + $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions); + $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestAssetBundle']->jsOptions['position']); + $this->assertArrayHasKey('position', $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions); + $this->assertEquals($pos, $view->assetBundles['yiiunit\\framework\\web\\TestJqueryAsset']->jsOptions['position']); + + switch($pos) + { + case View::POS_HEAD: + $expected = << + + +234 +EOF; + break; + case View::POS_BEGIN: + $expected = << +2 + +34 +EOF; + break; + default: + case View::POS_END: + $expected = << +23 + +4 +EOF; + break; + } + $this->assertEquals($expected, $view->renderFile('@yiiunit/data/views/rawlayout.php')); + } + + public function positionProvider2() + { + return array( + array(View::POS_BEGIN, true), + array(View::POS_BEGIN, false), + array(View::POS_END, true), + array(View::POS_END, false), + ); + } + + /** + * @dataProvider positionProvider + */ + public function testPositionDependencyConflict($pos, $jqAlreadyRegistered) + { + $view = $this->getView(); + + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestAssetBundle'] = array( + 'jsOptions' => array( + 'position' => $pos - 1, + ), + ); + $view->getAssetManager()->bundles['yiiunit\\framework\\web\\TestJqueryAsset'] = array( + 'jsOptions' => array( + 'position' => $pos, + ), + ); + + $this->assertEmpty($view->assetBundles); + if ($jqAlreadyRegistered) { + TestJqueryAsset::register($view); + } + $this->setExpectedException('yii\\base\\InvalidConfigException'); + TestAssetBundle::register($view); } }