-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Asset] Adding a new version strategy that reads from a manifest JSON file #22046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
e41cf49
1a3d465
6d19549
36fa8b3
0a99143
b2c7d32
9ed9c57
aa32896
99251c3
bebf674
5955f17
14d50e1
ff8869a
ada2471
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ CHANGELOG | |
3.3.0 | ||
----- | ||
|
||
* A new version strategy option called json_manifest_path was added | ||
that allows you to use the JsonManifestVersionStrategy. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. backticks here? |
||
* Added support for the `controller.service_arguments` tag, for injecting services into controllers' actions | ||
* Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) | ||
* Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -551,6 +551,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) | |
->scalarNode('version_strategy')->defaultNull()->end() | ||
->scalarNode('version')->defaultNull()->end() | ||
->scalarNode('version_format')->defaultValue('%%s?%%s')->end() | ||
->scalarNode('json_manifest_path')->defaultNull()->end() | ||
->scalarNode('base_path')->defaultValue('')->end() | ||
->arrayNode('base_urls') | ||
->requiresAtLeastOneElement() | ||
|
@@ -567,6 +568,18 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) | |
}) | ||
->thenInvalid('You cannot use both "version_strategy" and "version" at the same time under "assets".') | ||
->end() | ||
->validate() | ||
->ifTrue(function ($v) { | ||
return isset($v['version_strategy']) && isset($v['json_manifest_path']); | ||
}) | ||
->thenInvalid('You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets".') | ||
->end() | ||
->validate() | ||
->ifTrue(function ($v) { | ||
return isset($v['version']) && isset($v['json_manifest_path']); | ||
}) | ||
->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets".') | ||
->end() | ||
->fixXmlConfig('package') | ||
->children() | ||
->arrayNode('packages') | ||
|
@@ -582,6 +595,7 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) | |
->end() | ||
->end() | ||
->scalarNode('version_format')->defaultNull()->end() | ||
->scalarNode('json_manifest_path')->defaultNull()->end() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just asking: would Same for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a good thought. I don't know! It's a fairly easy thing to deprecate and change in the future anyways, and I can't see any other format in the near future. So let's go with what we like best right now. I changed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lgtm as json* |
||
->scalarNode('base_path')->defaultValue('')->end() | ||
->arrayNode('base_urls') | ||
->requiresAtLeastOneElement() | ||
|
@@ -598,6 +612,18 @@ private function addAssetsSection(ArrayNodeDefinition $rootNode) | |
}) | ||
->thenInvalid('You cannot use both "version_strategy" and "version" at the same time under "assets" packages.') | ||
->end() | ||
->validate() | ||
->ifTrue(function ($v) { | ||
return isset($v['version_strategy']) && isset($v['json_manifest_path']); | ||
}) | ||
->thenInvalid('You cannot use both "version_strategy" and "json_manifest_path" at the same time under "assets" packages.') | ||
->end() | ||
->validate() | ||
->ifTrue(function ($v) { | ||
return isset($v['version']) && isset($v['json_manifest_path']); | ||
}) | ||
->thenInvalid('You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.') | ||
->end() | ||
->end() | ||
->end() | ||
->end() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -810,7 +810,7 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co | |
if ($config['version_strategy']) { | ||
$defaultVersion = new Reference($config['version_strategy']); | ||
} else { | ||
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], '_default'); | ||
$defaultVersion = $this->createVersion($container, $config['version'], $config['version_format'], $config['json_manifest_path'], '_default'); | ||
} | ||
|
||
$defaultPackage = $this->createPackageDefinition($config['base_path'], $config['base_urls'], $defaultVersion); | ||
|
@@ -820,11 +820,14 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co | |
foreach ($config['packages'] as $name => $package) { | ||
if (null !== $package['version_strategy']) { | ||
$version = new Reference($package['version_strategy']); | ||
} elseif (!array_key_exists('version', $package)) { | ||
} elseif (!array_key_exists('version', $package) && null === $package['json_manifest_path']) { | ||
// if neither version nor json_manifest_path are specified, use the default | ||
$version = $defaultVersion; | ||
} else { | ||
// let format fallback to main version_format | ||
$format = $package['version_format'] ?: $config['version_format']; | ||
$version = $this->createVersion($container, $package['version'], $format, $name); | ||
$version = isset($package['version']) ? $package['version'] : null; | ||
$version = $this->createVersion($container, $version, $format, $package['json_manifest_path'], $name); | ||
} | ||
|
||
$container->setDefinition('assets._package_'.$name, $this->createPackageDefinition($package['base_path'], $package['base_urls'], $version)); | ||
|
@@ -856,20 +859,30 @@ private function createPackageDefinition($basePath, array $baseUrls, Reference $ | |
return $package; | ||
} | ||
|
||
private function createVersion(ContainerBuilder $container, $version, $format, $name) | ||
private function createVersion(ContainerBuilder $container, $version, $format, $jsonManifestPath, $name) | ||
{ | ||
if (null === $version) { | ||
return new Reference('assets.empty_version_strategy'); | ||
// Configuration prevents $version and $jsonManifestPath from being set | ||
if (null !== $version) { | ||
$def = new ChildDefinition('assets.static_version_strategy'); | ||
$def | ||
->replaceArgument(0, $version) | ||
->replaceArgument(1, $format) | ||
; | ||
$container->setDefinition('assets._version_'.$name, $def); | ||
|
||
return new Reference('assets._version_'.$name); | ||
} | ||
|
||
$def = new ChildDefinition('assets.static_version_strategy'); | ||
$def | ||
->replaceArgument(0, $version) | ||
->replaceArgument(1, $format) | ||
; | ||
$container->setDefinition('assets._version_'.$name, $def); | ||
if (null !== $jsonManifestPath) { | ||
$def = new ChildDefinition('assets.json_manifest_version_strategy'); | ||
$def->replaceArgument(0, $jsonManifestPath); | ||
$def->setPublic(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move to xml? |
||
$container->setDefinition('assets._version_'.$name, $def); | ||
|
||
return new Reference('assets._version_'.$name); | ||
} | ||
|
||
return new Reference('assets._version_'.$name); | ||
return new Reference('assets.empty_version_strategy'); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,11 @@ | ||
CHANGELOG | ||
========= | ||
|
||
3.3.0 | ||
----- | ||
* JsonManifestVersionStrategy was added as a way to read final, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added Json... |
||
versioned paths from a JSON manifest file. | ||
|
||
2.7.0 | ||
----- | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Asset\Tests\VersionStrategy; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy; | ||
|
||
class JsonManifestVersionStrategyTest extends TestCase | ||
{ | ||
public function testGetVersion() | ||
{ | ||
$strategy = $this->createStrategy('manifest-valid.json'); | ||
|
||
$this->assertEquals('main.123abc.js', $strategy->getVersion('main.js')); | ||
} | ||
|
||
public function testApplyVersion() | ||
{ | ||
$strategy = $this C45C ->createStrategy('manifest-valid.json'); | ||
|
||
$this->assertEquals('css/styles.555def.css', $strategy->getVersion('css/styles.css')); | ||
} | ||
|
||
public function testApplyVersionWhenKeyDoesNotExistInManifest() | ||
{ | ||
$strategy = $this->createStrategy('manifest-valid.json'); | ||
|
||
$this->assertEquals('css/other.css', $strategy->getVersion('css/other.css')); | ||
} | ||
|
||
/** | ||
* @expectedException \RuntimeException | ||
*/ | ||
public function testMissingManifestFileThrowsException() | ||
{ | ||
$strategy = $this->createStrategy('non-existent-file.json'); | ||
$strategy->getVersion('main.js'); | ||
} | ||
|
||
/** | ||
* @expectedException \RuntimeException | ||
* @expectedExceptionMessage Error parsing JSON | ||
*/ | ||
public function testManifestFileWithBadJSONThrowsException() | ||
{ | ||
$strategy = $this->createStrategy('manifest-invalid.json'); | ||
$strategy->getVersion('main.js'); | ||
} | ||
|
||
private function createStrategy($manifestFilename) | ||
{ | ||
return new JsonManifestVersionStrategy(__DIR__.'/../fixtures/'.$manifestFilename); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"main.js": main.123abc.js", | ||
"css/styles.css": "css/styles.555def.css" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a new ...