yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

registerJsFile with argument depends set does not use the position argument (it will always default to POS_END) #18192

Closed mikkelcp1 closed 4 years ago

mikkelcp1 commented 4 years ago

What steps will reproduce the problem?

$this->registerJsFile('@web/js/report-cards/colpo/summary/summary-chart.js', ['position' => \yii\web\View::POS_HEAD, 'depends' => [app\assets\HighChartsHeadAsset::className()]]);

What is the expected result?

The file '@web/js/report-cards/colpo/summary/summary-chart.js' should be included in the head (POS_HEAD)

What do you get instead?

The file '@web/js/report-cards/colpo/summary/summary-chart.js' is included in at the end of body (POS_END)

Additional info

See comment on pull: https://github.com/yiisoft/yii2/pull/16826#pullrequestreview-451928486

Fix;

--- ./vendor/yiisoft/yii2/web/View.php.org  2020-06-11 14:18:10.681826400 -0300
+++ ./vendor/yiisoft/yii2/web/View.php  2020-07-21 09:35:33.769180800 -0300
@@ -479,7 +479,7 @@
         $url = Yii::getAlias($url);
         $key = $key ?: $url;
         $depends = ArrayHelper::remove($options, 'depends', []);
-        $position = ArrayHelper::remove($options, 'position', self::POS_END);
+

         try {
             $asssetManagerAppendTimestamp = $this->getAssetManager()->appendTimestamp;
@@ -490,6 +490,7 @@
         $appendTimestamp = ArrayHelper::remove($options, 'appendTimestamp', $asssetManagerAppendTimestamp);

         if (empty($depends)) {
+            $position = ArrayHelper::remove($options, 'position', self::POS_END);
             // register directly without AssetManager
             if ($appendTimestamp && Url::isRelative($url) && ($timestamp = @filemtime(Yii::getAlias('@webroot/' . ltrim($url, '/'), false))) > 0) {
                 $url = $timestamp ? "$url?v=$timestamp" : $url;
Q A
Yii version 2.0.36
PHP version 7
Operating system
alex-code commented 4 years ago

@mikkelcp1 Your solution does fix the issue. Could you do a PR for this?