8000 [AssetMapper] Fixing incorrect exception & adding allowing more reali… · symfony/symfony@9f52bf1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9f52bf1

Browse files
weaverryannicolas-grekas
weaverryan
authored and
nicolas-grekas
committed
[AssetMapper] Fixing incorrect exception & adding allowing more realistic error mode
1 parent 2979e1e commit 9f52bf1

16 files changed

+152
-38
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -870,9 +870,10 @@ private function addAssetMapperSection(ArrayNodeDefinition $rootNode, callable $
870870
->info('The public path where the assets will be written to (and served from when "server" is true)')
871871
->defaultValue('/assets/')
872872
->end()
873-
->booleanNode('strict_mode')
874-
->info('If true, an exception will be thrown if an asset cannot be found when imported from JavaScript or CSS files - e.g. "import \'./non-existent.js\'"')
875-
->defaultValue(true)
873+
->enumNode('missing_import_mode')
874+
->values(['strict', 'warn', 'ignore'])
875+
->info('Behavior if an asset cannot be found when imported from JavaScript or CSS files - e.g. "import \'./non-existent.js\'". "strict" means an exception is thrown, "warn" means a warning is logged, "ignore" means the import is left as-is.')
876+
->defaultValue('warn')
876877
->end()
877878
->arrayNode('extensions')
878879
->info('Key-value pair of file extensions set to their mime type.')

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,10 +1305,10 @@ private function registerAssetMapperConfiguration(array $config, ContainerBuilde
13051305
}
13061306

13071307
$container->getDefinition('asset_mapper.compiler.css_asset_url_compiler')
1308-
->setArgument(0, $config['strict_mode']);
1308+
->setArgument(0, $config['missing_import_mode']);
13091309

13101310
$container->getDefinition('asset_mapper.compiler.javascript_import_path_compiler')
1311-
->setArgument(0, $config['strict_mode']);
1311+
->setArgument(0, $config['missing_import_mode']);
13121312

13131313
$container
13141314
->getDefinition('asset_mapper.importmap.manager')

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@
114114

115115
->set('asset_mapper.compiler.css_asset_url_compiler', CssAssetUrlCompiler::class)
116116
->args([
117-
abstract_arg('strict mode'),
117+
abstract_arg('missing import mode'),
118+
service('logger'),
118119
])
119120
->tag('asset_mapper.compiler')
120121

@@ -123,7 +124,8 @@
123124

124125
->set('asset_mapper.compiler.javascript_import_path_compiler', JavaScriptImportPathCompiler::class)
125126
->args([
126-
abstract_arg('strict mode'),
127+
abstract_arg('missing import mode'),
128+
service('logger'),
127129
])
128130
->tag('asset_mapper.compiler')
129131

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@
197197
<xsd:attribute name="enabled" type="xsd:boolean" />
198198
<xsd:attribute name="server" type="xsd:boolean" />
199199
<xsd:attribute name="public-prefix" type="xsd:string" />
200-
<xsd:attribute name="strict-mode" type="xsd:boolean" />
200+
<xsd:attribute name="missing-import-mode" type="missing-import-mode" />
201201
<xsd:attribute name="importmap-path" type="xsd:string" />
202202
<xsd:attribute name="importmap-polyfill" type="xsd:string" />
203203
<xsd:attribute name="vendor-dir" type="xsd:string" />
@@ -216,6 +216,14 @@
216216
<xsd:attribute name="key" type="xsd:string" use="required" />
217217
</xsd:complexType>
218218

219+
<xsd:simpleType name="missing-import-mode">
220+
<xsd:restriction base="xsd:string">
221+
<xsd:enumeration value="strict" />
222+
<xsd:enumeration value="warn" />
223+
<xsd:enumeration value="ignore" />
224+
</xsd:restriction>
225+
</xsd:simpleType>
226+
219227
<xsd:complexType name="translator">
220228
<xsd:sequence>
221229
<xsd:element name="fallback" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public function testAssetMapperCanBeEnabled()
107107
'excluded_patterns' => [],
108108
'server' => true,
109109
'public_prefix' => '/assets/',
110-
'strict_mode' => true,
110+
'missing_import_mode' => 'warn',
111111
'extensions' => [],
112112
'importmap_path' => '%kernel.project_dir%/importmap.php',
113113
'importmap_polyfill' => null,
@@ -619,7 +619,7 @@ protected static function getBundleDefaultConfig()
619619
'excluded_patterns' => [],
620620
'server' => true,
621621
'public_prefix' => '/assets/',
622-
'strict_mode' => true,
622+
'missing_import_mode' => 'warn',
623623
'extensions' => [],
624624
'importmap_path' => '%kernel.project_dir%/importmap.php',
625625
'importmap_polyfill' => null,

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/asset_mapper.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
enabled="true"
1212
server="true"
1313
public-prefix="/assets_path/"
14-
strict-mode="true"
14+
missing-import-mode="strict"
1515
importmap-path="%kernel.project_dir%/importmap.php"
1616
importmap-polyfill="https://cdn.example.com/polyfill.js"
1717
vendor-dir="%kernel.project_dir%/assets/vendor"

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/XmlFrameworkExtensionTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,8 @@ public function testAssetMapper()
8989

9090
$definition = $container->getDefinition('asset_mapper.repository');
9191
$this->assertSame(['assets/' => '', 'assets2/' => 'my_namespace'], $definition->getArgument(0));
92+
93+
$definition = $container->getDefinition('asset_mapper.compiler.css_asset_url_compiler');
94+
$this->assertSame('strict', $definition->getArgument(0));
9295
}
9396
}

‎src/Symfony/Component/AssetMapper/Compiler/AssetCompilerInterface.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323
*/
2424
interface AssetCompilerInterface
2525
{
26+
public const MISSING_IMPORT_STRICT = 'strict';
27+
public const MISSING_IMPORT_WARN = 'warn';
28+
public const MISSING_IMPORT_IGNORE = 'ignore';
29+
2630
public function supports(MappedAsset $asset): bool;
2731

2832
/**

‎src/Symfony/Component/AssetMapper/Compiler/AssetCompilerPathResolverTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace Symfony\Component\AssetMapper\Compiler;
1313

14-
use Symfony\Component\Asset\Exception\RuntimeException;
14+
use Symfony\Component\AssetMapper\Exception\RuntimeException;
1515

1616
/**
1717
* Helps resolve "../" and "./" in paths.

‎src/Symfony/Component/AssetMapper/Compiler/CssAssetUrlCompiler.php

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@
1111

1212
namespace Symfony\Component\AssetMapper\Compiler;
1313

14+
use Psr\Log\LoggerInterface;
15+
use Psr\Log\NullLogger;
1416
use Symfony\Component\AssetMapper\AssetDependency;
1517
use Symfony\Component\AssetMapper\AssetMapperInterface;
18+
use Symfony\Component\AssetMapper\Exception\RuntimeException;
1619
use Symfony\Component\AssetMapper\MappedAsset;
1720

1821
/**
@@ -26,23 +29,32 @@ final class CssAssetUrlCompiler implements AssetCompilerInterface
2629
{
2730
use AssetCompilerPathResolverTrait;
2831

32+
private readonly LoggerInterface $logger;
33+
2934
// https://regex101.com/r/BOJ3vG/1
3035
public const ASSET_URL_PATTERN = '/url\(\s*["\']?(?!(?:\/|\#|%23|data|http|\/\/))([^"\'\s?#)]+)([#?][^"\')]+)?\s*["\']?\)/';
3136

32-
public function __construct(private readonly bool $strictMode = true)
33-
{
37+
public function __construct(
38+
private readonly string $missingImportMode = self::MISSING_IMPORT_WARN,
39+
LoggerInterface $logger = null,
40+
) {
41+
$this->logger = $logger ?? new NullLogger();
3442
}
3543

3644
public function compile(string $content, MappedAsset $asset, AssetMapperInterface $assetMapper): string
3745
{
3846
return preg_replace_callback(self::ASSET_URL_PATTERN, function ($matches) use ($asset, $assetMapper) {
39-
$resolvedPath = $this->resolvePath(\dirname($asset->getLogicalPath()), $matches[1]);
47+
try {
48+
$resolvedPath = $this->resolvePath(\dirname($asset->getLogicalPath()), $matches[1]);
49+
} catch (RuntimeException $e) {
50+
$this->handleMissingImport(sprintf('Error processing import in "%s": "%s"', $asset->getSourcePath(), $e->getMessage()), $e);
51+
52+
return $matches[0];
53+
}
4054
$dependentAsset = $assetMapper->getAsset($resolvedPath);
4155

4256
if (null === $dependentAsset) {
43-
if ($this->strictMode) {
44-
throw new \RuntimeException(sprintf('Unable to find asset "%s" referenced in "%s".', $resolvedPath, $asset->getSourcePath()));
45-
}
57+
$this->handleMissingImport(sprintf('Unable to find asset "%s" referenced in "%s".', $matches[1], $asset->getSourcePath()));
4658

4759
// return original, unchanged path
4860
return $matches[0];
@@ -59,4 +71,13 @@ public function supports(MappedAsset $asset): bool
5971
{
6072
return 'css' === $asset->getPublicExtension();
6173
}
74+
75+
private function handleMissingImport(string $message, \Throwable $e = null): void
76+
{
77+
match ($this->missingImportMode) {
78+
AssetCompilerInterface::MISSING_IMPORT_IGNORE => null,
79+
AssetCompilerInterface::MISSING_IMPORT_WARN => $this->logger->warning($message),
80+
AssetCompilerInterface::MISSING_IMPORT_STRICT => throw new RuntimeException($message, 0, $e),
81+
};
82+
}
6283
}

‎src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@
1111

1212
namespace Symfony\Component\AssetMapper\Compiler;
1313

14+
use Psr\Log\LoggerInterface;
15+
use Psr\Log\NullLogger;
1416
use Symfony\Component\AssetMapper\AssetDependency;
1517
use Symfony\Component\AssetMapper\AssetMapperInterface;
18+
use Symfony\Component\AssetMapper\Exception\RuntimeException;
1619
use Symfony\Component\AssetMapper\MappedAsset;
1720

1821
/**
@@ -26,31 +29,44 @@ final class JavaScriptImportPathCompiler implements AssetCompilerInterface
2629
{
2730
use AssetCompilerPathResolverTrait;
2831

32+
private readonly LoggerInterface $logger;
33+
2934
// https://regex101.com/r/VFdR4H/1
3035
private const IMPORT_PATTERN = '/(?:import\s+(?:(?:\*\s+as\s+\w+|[\w\s{},*]+)\s+from\s+)?|\bimport\()\s*[\'"`](\.\/[^\'"`]+|(\.\.\/)+[^\'"`]+)[\'"`]\s*[;\)]?/m';
3136

32-
public function __construct(private readonly bool $strictMode = true)
33-
{
37+
public function __construct(
38+
private readonly string $missingImportMode = self::MISSING_IMPORT_WARN,
39+
LoggerInterface $logger = null,
40+
) {
41+
$this->logger = $logger ?? new NullLogger();
3442
}
3543

3644
public function compile(string $content, MappedAsset $asset, AssetMapperInterface $assetMapper): string
3745
{
3846
return preg_replace_callback(self::IMPORT_PATTERN, function ($matches) use ($asset, $assetMapper) {
39-
$resolvedPath = $this->resolvePath(\dirname($asset->getLogicalPath()), $matches[1]);
47+
try {
48+
$resolvedPath = $this->resolvePath(\dirname($asset->getLogicalPath()), $matches[1]);
49+
} catch (RuntimeException $e) {
50+
$this->handleMissingImport(sprintf('Error processing import in "%s": "%s"', $asset->getSourcePath(), $e->getMessage()), $e);
51+
52+
return $matches[0];
53+
}
4054

4155
$dependentAsset = $assetMapper->getAsset($resolvedPath);
4256

43-
if (!$dependentAsset && $this->strictMode) {
44-
$message = sprintf('Unable to find asset "%s" imported from "%s".', $resolvedPath, $asset->getSourcePath());
57+
if (!$dependentAsset) {
58+
$message = sprintf('Unable to find asset "%s" imported from "%s".', $matches[1], $asset->getSourcePath());
4559

4660
if (null !== $assetMapper->getAsset(sprintf('%s.js', $resolvedPath))) {
47-
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $resolvedPath);
61+
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $matches[1]);
4862
}
4963

50-
throw new \RuntimeException($message);
64+
$this->handleMissingImport($message);
65+
66+
return $matches[0];
5167
}
5268

53-
if ($dependentAsset && $this->supports($dependentAsset)) {
69+
if ($this->supports($dependentAsset)) {
5470
// If we found the path and it's a JavaScript file, list it as a dependency.
5571
// This will cause the asset to be included in the importmap.
5672
$isLazy = str_contains($matches[0], 'import(');
@@ -80,4 +96,20 @@ private function makeRelativeForJavaScript(string $path): string
8096

8197
return './'.$path;
8298
}
99+
100+
private function handleMissingImport(string $message, \Throwable $e = null): void
101+
{
102+
switch ($this->missingImportMode) {
103+
case AssetCompilerInterface::MISSING_IMPORT_IGNORE:
104+
return;
105+
106+
case AssetCompilerInterface::MISSING_IMPORT_WARN:
107+
$this->logger->warning($message);
108+
109+
return;
110+
111+
case AssetCompilerInterface::MISSING_IMPORT_STRICT:
112+
throw new RuntimeException($message, 0, $e);
113+
}
114+
}
83115
}

‎src/Symfony/Component/AssetMapper/Exception/ExceptionInterface.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\AssetMapper\Exception;
13+
14+
interface ExceptionInterface extends \Throwable
15+
{
16+
}

‎src/Symfony/Component/AssetMapper/Exception/RuntimeException.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\AssetMapper\Exception;
13+
14+
class RuntimeException extends \RuntimeException implements ExceptionInterface
15+
{
16+
}

‎src/Symfony/Component/AssetMapper/Tests/Compiler/AssetCompilerPathResolverTraitTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
namespace Symfony\Component\AssetMapper\Tests\Compiler;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\Asset\Exception\RuntimeException;
1615
use Symfony\Component\AssetMapper\Compiler\AssetCompilerPathResolverTrait;
16+
use Symfony\Component\AssetMapper\Exception\RuntimeException;
1717

1818
class AssetCompilerPathResolverTraitTest extends TestCase
1919
{

‎src/Symfony/Component/AssetMapper/Tests/Compiler/CssAssetUrlCompilerTest.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
namespace Symfony\Component\AssetMapper\Tests\Compiler;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Psr\Log\LoggerInterface;
1516
use Symfony\Component\AssetMapper\AssetDependency;
1617
use Symfony\Component\AssetMapper\AssetMapperInterface;
18+
use Symfony\Component\AssetMapper\Compiler\AssetCompilerInterface;
1719
use Symfony\Component\AssetMapper\Compiler\CssAssetUrlCompiler;
1820
use Symfony\Component\AssetMapper\MappedAsset;
1921

@@ -24,8 +26,9 @@ class CssAssetUrlCompilerTest extends TestCase
2426
*/
2527
public function testCompile(string $sourceLogicalName, string $input, string $expectedOutput, array $expectedDependencies)
2628
{
27-
$compiler = new CssAssetUrlCompiler(false);
29+
$compiler = new CssAssetUrlCompiler(AssetCompilerInterface::MISSING_IMPORT_IGNORE, $this->createMock(LoggerInterface::class));
2830
$asset = new MappedAsset($sourceLogicalName);
31+
$asset->setSourcePath('anything');
2932
$asset->setPublicPathWithoutDigest('/assets/'.$sourceLogicalName);
3033
$this->assertSame($expectedOutput, $compiler->compile($input, $asset, $this->createAssetMapper()));
3134
$assetDependencyLogicalPaths = array_map(fn (AssetDependency $dependency) => $dependency->asset->getLogicalPath(), $asset->getDependencies());
@@ -117,7 +120,7 @@ public function testStrictMode(string $sourceLogicalName, string $input, ?string
117120
$asset = new MappedAsset($sourceLogicalName);
118121
$asset->setSourcePath('/path/to/styles.css');
119122

120-
$compiler = new CssAssetUrlCompiler(true);
123+
$compiler = new CssAssetUrlCompiler(AssetCompilerInterface::MISSING_IMPORT_STRICT, $this->createMock(LoggerInterface::class));
121124
$this->assertSame($input, $compiler->compile($input, $asset, $this->createAssetMapper()));
122125
}
123126

0 commit comments

Comments
 (0)
0