Skip to content

Commit

Permalink
Fixes #2445 pipeline excluded assets with cache on
Browse files Browse the repository at this point in the history
  • Loading branch information
@rhukster
rhukster committed Apr 13, 2019
1 parent 08423df commit 8fd7a5a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 44 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 18 additions & 12 deletions system/src/Grav/Common/Assets.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand Down Expand Up @@ -264,6 +266,21 @@ public function registerCollection($collectionName, Array $assets, $overwrite =
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;
});
Expand Down Expand Up @@ -292,11 +309,9 @@ public function render($type, $group = 'head', $attributes = [])
$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);
Expand All @@ -309,22 +324,13 @@ public function render($type, $group = 'head', $attributes = [])
$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();
Expand Down
6 changes: 6 additions & 0 deletions system/src/Grav/Common/Assets/BaseAsset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ public function getRemote()
return $this->remote;
}

public function setPosition($position)
{
$this->position = $position;
return $this;
}


/**
*
Expand Down
33 changes: 6 additions & 27 deletions system/src/Grav/Common/Assets/Pipeline.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -91,11 +88,10 @@ public function __construct(array $elements = [], ?string $key = null)
* @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;
Expand All @@ -119,21 +115,13 @@ public function renderCss($assets, $group, $attributes = [], &$no_pipeline = [])
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')) {
Expand Down Expand Up @@ -164,11 +152,10 @@ public function renderCss($assets, $group, $attributes = [], &$no_pipeline = [])
* @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;
Expand All @@ -192,21 +179,13 @@ public function renderJs($assets, $group, $attributes = [], &$no_pipeline = [])
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')) {
Expand Down
6 changes: 1 addition & 5 deletions system/src/Grav/Common/Assets/Traits/AssetUtilsTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,10 @@ public static function isRemoteLink($link)
*
* @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 = '';

Expand Down Expand Up @@ -74,9 +73,6 @@ protected function gatherLinks(array $assets, $css = true, &$no_pipeline = [])

// 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;
}

Expand Down

0 comments on commit 8fd7a5a

Please sign in to comment.