From 8fd7a5aebe34e4e5f31c769e7aac1e25bdc618df Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Sat, 13 Apr 2019 13:18:13 -0600 Subject: [PATCH] Fixes #2445 pipeline excluded assets with cache on --- CHANGELOG.md | 1 + system/src/Grav/Common/Assets.php | 30 ++++++++++------- system/src/Grav/Common/Assets/BaseAsset.php | 6 ++++ system/src/Grav/Common/Assets/Pipeline.php | 33 ++++--------------- .../Common/Assets/Traits/AssetUtilsTrait.php | 6 +--- 5 files changed, 32 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dec33462..5fd5ffa57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## mm/dd/2019 1. [](#bugfix) + * Rework logic to pull out excluded files from pipeline more reliably [#2445](https://github.com/getgrav/grav/issues/2445) * Better logic in `Utils::normalizePath` to handle externals properly [#2216](https://github.com/getgrav/grav/issues/2216) # v1.6.3 diff --git a/system/src/Grav/Common/Assets.php b/system/src/Grav/Common/Assets.php index 6c3cf20b9..af54b26bb 100644 --- a/system/src/Grav/Common/Assets.php +++ b/system/src/Grav/Common/Assets.php @@ -45,8 +45,10 @@ class Assets extends PropertyObject // Config Options protected $css_pipeline; + protected $css_pipeline_include_externals; protected $css_pipeline_before_excludes; protected $js_pipeline; + protected $js_pipeline_include_externals; protected $js_pipeline_before_excludes; protected $pipeline_options = []; @@ -264,6 +266,21 @@ class Assets extends PropertyObject protected function filterAssets($assets, $key, $value, $sort = false) { $results = array_filter($assets, function($asset) use ($key, $value) { + + if ($key === 'position' && $value === 'pipeline') { + + $type = $asset->getType(); + + if ($asset->getRemote() && $this->{$type . '_pipeline_include_externals'} === false) { + if ($this->{$type . '_pipeline_before_excludes'}) { + $asset->setPosition('after'); + } else { + $asset->setPosition('before'); + } + return false; + } + + } if ($asset[$key] === $value) return true; return false; }); @@ -292,11 +309,9 @@ class Assets extends PropertyObject $before_output = ''; $pipeline_output = ''; $after_output = ''; - $no_pipeline = []; $assets = 'assets_' . $type; $pipeline_enabled = $type . '_pipeline'; - $before_excludes = $type . '_pipeline_before_excludes'; $render_pipeline = 'render' . ucfirst($type); $group_assets = $this->filterAssets($this->$assets, 'group', $group); @@ -309,22 +324,13 @@ class Assets extends PropertyObject $options = array_merge($this->pipeline_options, ['timestamp' => $this->timestamp]); $pipeline = new Pipeline($options); - $pipeline_output = $pipeline->$render_pipeline($pipeline_assets, $group, $attributes, $no_pipeline); + $pipeline_output = $pipeline->$render_pipeline($pipeline_assets, $group, $attributes); } else { foreach ($pipeline_assets as $asset) { $pipeline_output .= $asset->render(); } } - // Handle stuff that couldn't be pipelined - if (!empty($no_pipeline)) { - if ($this->{$before_excludes}) { - $after_assets = array_merge($after_assets, $no_pipeline); - } else { - $before_assets = array_merge($before_assets, $no_pipeline); - } - } - // Before Pipeline foreach ($before_assets as $asset) { $before_output .= $asset->render(); diff --git a/system/src/Grav/Common/Assets/BaseAsset.php b/system/src/Grav/Common/Assets/BaseAsset.php index 802367c7f..98f0f0560 100644 --- a/system/src/Grav/Common/Assets/BaseAsset.php +++ b/system/src/Grav/Common/Assets/BaseAsset.php @@ -129,6 +129,12 @@ abstract class BaseAsset extends PropertyObject return $this->remote; } + public function setPosition($position) + { + $this->position = $position; + return $this; + } + /** * diff --git a/system/src/Grav/Common/Assets/Pipeline.php b/system/src/Grav/Common/Assets/Pipeline.php index 9557030cb..b009585c8 100644 --- a/system/src/Grav/Common/Assets/Pipeline.php +++ b/system/src/Grav/Common/Assets/Pipeline.php @@ -50,9 +50,6 @@ class Pipeline extends PropertyObject protected $query; protected $asset; - protected $css_pipeline_include_externals; - protected $js_pipeline_include_externals; - /** * Closure used by the pipeline to fetch assets. * @@ -91,11 +88,10 @@ class Pipeline extends PropertyObject * @param array $assets * @param string $group * @param array $attributes - * @param array $no_pipeline * * @return bool|string URL or generated content if available, else false */ - public function renderCss($assets, $group, $attributes = [], &$no_pipeline = []) + public function renderCss($assets, $group, $attributes = []) { // temporary list of assets to pipeline $inline_group = false; @@ -119,21 +115,13 @@ class Pipeline extends PropertyObject if (file_exists($this->assets_dir . $file)) { $buffer = file_get_contents($this->assets_dir . $file) . "\n"; } else { - - foreach ($assets as $id => $asset) { - if ($this->css_pipeline_include_externals === false && $asset->getRemote()) { - $no_pipeline[$id] = $asset; - unset($assets[$id]); - } - } - //if nothing found get out of here! - if (empty($assets) && empty($no_pipeline)) { + if (empty($assets)) { return false; } // Concatenate files - $buffer = $this->gatherLinks($assets, self::CSS_ASSET, $no_pipeline); + $buffer = $this->gatherLinks($assets, self::CSS_ASSET); // Minify if required if ($this->shouldMinify('css')) { @@ -164,11 +152,10 @@ class Pipeline extends PropertyObject * @param array $assets * @param string $group * @param array $attributes - * @param array $no_pipeline * * @return bool|string URL or generated content if available, else false */ - public function renderJs($assets, $group, $attributes = [], &$no_pipeline = []) + public function renderJs($assets, $group, $attributes = []) { // temporary list of assets to pipeline $inline_group = false; @@ -192,21 +179,13 @@ class Pipeline extends PropertyObject if (file_exists($this->assets_dir . $file)) { $buffer = file_get_contents($this->assets_dir . $file) . "\n"; } else { - - foreach ($assets as $id => $asset) { - if ($this->js_pipeline_include_externals === false && $asset->getRemote()) { - $no_pipeline[$id] = $asset; - unset($assets[$id]); - } - } - //if nothing found get out of here! - if (empty($assets) && empty($no_pipeline)) { + if (empty($assets)) { return false; } // Concatenate files - $buffer = $this->gatherLinks($assets, self::JS_ASSET, $no_pipeline); + $buffer = $this->gatherLinks($assets, self::JS_ASSET); // Minify if required if ($this->shouldMinify('js')) { diff --git a/system/src/Grav/Common/Assets/Traits/AssetUtilsTrait.php b/system/src/Grav/Common/Assets/Traits/AssetUtilsTrait.php index c478b1aae..0a1d149de 100644 --- a/system/src/Grav/Common/Assets/Traits/AssetUtilsTrait.php +++ b/system/src/Grav/Common/Assets/Traits/AssetUtilsTrait.php @@ -38,11 +38,10 @@ trait AssetUtilsTrait * * @param array $assets * @param bool $css - * @param array $no_pipeline * * @return string */ - protected function gatherLinks(array $assets, $css = true, &$no_pipeline = []) + protected function gatherLinks(array $assets, $css = true) { $buffer = ''; @@ -74,9 +73,6 @@ trait AssetUtilsTrait // No file found, skip it... if ($file === false) { - if (!$local) { // Assume we couldn't download this file for some reason assume it's not pipeline compatible - $no_pipeline[$id] = $asset; - } continue; }