8000 Merge branch '6.4' into 7.0 · jschaedl/symfony@fabd27a · GitHub
[go: up one dir, main page]
Skip to content

Commit fabd27a

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.0
* 6.4: [RateLimiter] CompoundLimiter was accepting requests even when some limiters already consumed all tokens [AssetMapper] Only download a CSS file if it is explicitly advertised [AssetMapper] Avoid storing & caching MappedAsset objects [AssetMapper] Improving exception if a vendor asset's path is not mapped [AssetMapper] Adding import regex support for not importing any value [AssetMapper] If assets are served from a subdirectory or CDN, also adjust importmap keys
2 parents a1fa0fa + 151dba0 commit fabd27a

16 files changed

+179
-80
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
7272
$addToImportMap = $isRelativeImport;
7373
$asset->addJavaScriptImport(new JavaScriptImport(
7474
$addToImportMap ? $dependentAsset->publicPathWithoutDigest : $importedModule,
75-
$dependentAsset,
75+
$dependentAsset->logicalPath,
76+
$dependentAsset->sourcePath,
7677
$isLazy,
7778
$addToImportMap,
7879
));

‎src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ private function collectResourcesFromAsset(MappedAsset $mappedAsset): array
6969
}
7070

7171
foreach ($mappedAsset->getJavaScriptImports() as $import) {
72-
$resources[] = new FileExistenceResource($import->asset->sourcePath);
72+
$resources[] = new Fil 10000 eExistenceResource($import->assetSourcePath);
7373
}
7474

7575
return $resources;

‎src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,15 @@ private function addImplicitEntries(ImportMapEntry $entry, array $currentImportE
179179

180180
// check if this import requires an automatic importmap entry
181181
if ($javaScriptImport->addImplicitlyToImportMap) {
182+
if (!$importedAsset = $this->assetMapper->getAsset($javaScriptImport->assetLogicalPath)) {
183+
// should not happen at this point, unless something added a bogus JavaScriptImport to this asset
184+
throw new LogicException(sprintf('Cannot find imported JavaScript asset "%s" in asset mapper.', $javaScriptImport->assetLogicalPath));
185+
}
186+
182187
$nextEntry = ImportMapEntry::createLocal(
183188
$importName,
184-
ImportMapType::tryFrom($javaScriptImport->asset->publicExtension) ?: ImportMapType::JS,
185-
$javaScriptImport->asset->logicalPath,
189+
ImportMapType::tryFrom($importedAsset->publicExtension) ?: ImportMapType::JS,
190+
$importedAsset->logicalPath,
186191
false,
187192
);
188193

@@ -223,7 +228,11 @@ private function findEagerImports(MappedAsset $asset): array
223228
$dependencies[] = $javaScriptImport->importName;
224229

225230
// Follow its imports!
226-
$dependencies = array_merge($dependencies, $this->findEagerImports($javaScriptImport->asset));
231+
if (!$nextAsset = $this->assetMapper->getAsset($javaScriptImport->assetLogicalPath)) {
232+
// should not happen at this point, unless something added a bogus JavaScriptImport to this asset
233+
throw new LogicException(sprintf('Cannot find imported JavaScript asset "%s" in asset mapper.', $javaScriptImport->assetLogicalPath));
234+
}
235+
$dependencies = array_merge($dependencies, $this->findEagerImports($nextAsset));
227236
}
228237

229238
return $dependencies;
@@ -232,7 +241,11 @@ private function findEagerImports(MappedAsset $asset): array
232241
private function createMissingImportMapAssetException(ImportMapEntry $entry): \InvalidArgumentException
233242
{
234243
if ($entry->isRemotePackage()) {
235-
throw new LogicException(sprintf('The "%s" vendor asset is missing. Try running the "importmap:install" command.', $entry->importName));
244+
if (!is_file($entry->path)) {
245+
throw new LogicException(sprintf('The "%s" vendor asset is missing. Try running the "importmap:install" command.', $entry->importName));
246+
}
247+
248+
throw new LogicException(sprintf('The "%s" vendor file exists locally (%s), but cannot be found in any asset map paths. Be sure the assets vendor directory is an asset mapper path.', $entry->importName, $entry->path));
236249
}
237250

238251
throw new LogicException(sprintf('The asset "%s" cannot be found in any asset map paths.', $entry->path));

‎src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ public function render(string|array $entryPoint, array $attributes = []): string
6262
continue;
6363
}
6464

65+
// for subdirectories or CDNs, the import name needs to be the full URL
66+
if (str_starts_with($importName, '/') && $this->assetPackages) {
67+
$importName = $this->assetPackages->getUrl(ltrim($importName, '/'));
68+
}
69+
6570
$preload = $data['preload'] ?? false;
6671
if ('css' !== $data['type']) {
6772
$importMap[$importName] = $path;

‎src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,20 @@
1111

1212
namespace Symfony\Component\AssetMapper\ImportMap;
1313

14-
use Symfony\Component\AssetMapper\MappedAsset;
15-
1614
/**
1715
* Represents a module that was imported by a JavaScript file.
1816
*/
1917
final class JavaScriptImport
2018
{
2119
/**
22-
* @param string $importName The name of the import needed in the importmap, e.g. "/foo.js" or "react"
23-
* @param MappedAsset $asset The asset that was imported
24-
* @param bool $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
20+
* @param string $importName The name of the import needed in the importmap, e.g. "/foo.js" or "react"
21+
* @param string $assetLogicalPath Logical path to the mapped ass that was imported
22+
* @param bool $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
2523
*/
2624
public function __construct(
2725
public readonly string $importName,
28-
public readonly MappedAsset $asset,
26+
public readonly string $assetLogicalPath,
27+
public readonly string $assetSourcePath,
2928
public readonly bool $isLazy = false,
3029
public bool $addImplicitlyToImportMap = false,
3130
) {

‎src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class JsDelivrEsmResolver implements PackageResolverInterface
2626
public const URL_PATTERN_DIST = self::URL_PATTERN_DIST_CSS.'/+esm';
2727
public const URL_PATTERN_ENTRYPOINT = 'https://data.jsdelivr.com/v1/packages/npm/%s@%s/entrypoints';
2828

29-
public const IMPORT_REGEX = '{from"/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm"}';
29+
public const IMPORT_REGEX = '#(?:import\s*(?:(?:\{[^}]*\}|\w+|\*\s*as\s+\w+)\s*\bfrom\s*)?|export\s*(?:\{[^}]*\}|\*)\s*from\s*)("/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm")#';
3030

3131
private HttpClientInterface $httpClient;
3232

@@ -129,8 +129,9 @@ public function resolvePackages(array $packagesToRequire): array
129129

130130
$entrypoints = $cssEntrypointResponse->toArray()['entrypoints'] ?? [];
131131
$cssFile = $entrypoints['css']['file'] ?? null;
132+
$guessed = $entrypoints['css']['guessed'] ?? true;
132133

133-
if (!$cssFile) {
134+
if (!$cssFile || $guessed) {
134135
continue;
135136
}
136137

@@ -221,9 +222,9 @@ private function fetchPackageRequirementsFromImports(string $content): array
221222
// imports from jsdelivr follow a predictable format
222223
preg_match_all(self::IMPORT_REGEX, $content, $matches);
223224
$dependencies = [];
224-
foreach ($matches[1] as $index => $packageName) {
225-
$version = $matches[2][$index] ?: null;
226-
$packageName .= $matches[3][$index]; // add the path if any
225+
foreach ($matches[2] as $index => $packageName) {
226+
$version = $matches[3][$index] ?: null;
227+
$packageName .= $matches[4][$index]; // add the path if any
227228

228229
$dependencies[] = new PackageRequireOptions($packageName, $version);
229230
}
@@ -239,10 +240,11 @@ private function fetchPackageRequirementsFromImports(string $content): array
239240
private function makeImportsBare(string $content, array &$dependencies): string
240241
{
241242
$content = preg_replace_callback(self::IMPORT_REGEX, function ($matches) use (&$dependencies) {
242-
$packageName = $matches[1].$matches[3]; // add the path if any
243+
$packageName = $matches[2].$matches[4]; // add the path if any
243244
$dependencies[] = $packageName;
244245

245-
return sprintf('from"%s"', $packageName);
246+
// replace the "/npm/package@version/+esm" with "package@version"
247+
return str_replace($matches[1], sprintf('"%s"', $packageName), $matches[0]);
246248
}, $content);
247249

248250
// source maps are not also downloaded - so remove the sourceMappingURL

‎src/Symfony/Component/AssetMapper/MappedAsset.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ public function __construct(
9494
}
9595

9696
/**
97+
* Assets that the content of this asset depends on - for internal caching.
98+
*
9799
* @return MappedAsset[]
98100
*/
99101
public function getDependencies(): array

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

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
5858
->method('getAsset')
5959
->willReturnCallback(function ($path) {
6060
return match ($path) {
61-
'module_in_importmap_local_asset.js' => new MappedAsset('module_in_importmap_local_asset.js', publicPathWithoutDigest: '/assets/module_in_importmap_local_asset.js'),
61+
'module_in_importmap_local_asset.js' => new MappedAsset('module_in_importmap_local_asset.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/module_in_importmap_local_asset.js'),
6262
default => null,
6363
};
6464
});
@@ -67,11 +67,11 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
6767
->method('getAssetFromSourcePath')
6868
->willReturnCallback(function ($path) {
6969
return match ($path) {
70-
'/project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
71-
'/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
72-
'/project/assets/styles.css' => new MappedAsset('styles.css', publicPathWithoutDigest: '/assets/styles.css'),
73-
'/project/assets/vendor/module_in_importmap_remote.js' => new MappedAsset('module_in_importmap_remote.js', publicPathWithoutDigest: '/assets/module_in_importmap_remote.js'),
74-
'/project/assets/vendor/@popperjs/core.js' => new MappedAsset('assets/vendor/@popperjs/core.js', publicPathWithoutDigest: '/assets/@popperjs/core.js'),
70+
'/project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
71+
'/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
72+
'/project/assets/styles.css' => new MappedAsset('styles.css', '/can/be/anything.js', publicPathWithoutDigest: '/assets/styles.css'),
73+
'/project/assets/vendor/module_in_importmap_remote.js' => new MappedAsset('module_in_importmap_remote.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/module_in_importmap_remote.js'),
74+
'/project/assets/vendor/@popperjs/core.js' => new MappedAsset('assets/vendor/@popperjs/core.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/@popperjs/core.js'),
7575
default => null,
7676
};
7777
});
@@ -81,7 +81,7 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
8181
$this->assertSame($input, $compiler->compile($input, $asset, $assetMapper));
8282
$actualImports = [];
8383
foreach ($asset->getJavaScriptImports() as $import) {
84-
$actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->asset->logicalPath, 'add' => $import->addImplicitlyToImportMap];
84+
$actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->assetLogicalPath, 'add' => $import->addImplicitlyToImportMap];
8585
}
8686

8787
$this->assertEquals($expectedJavaScriptImports, $actualImports);
@@ -304,9 +304,9 @@ public function testCompileFindsRelativePathsViaSourcePath()
304304
->method('getAssetFromSourcePath')
305305
->willRe 341A turnCallback(function ($path) {
306306
return match ($path) {
307-
'/project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
308-
'/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
309-
'/project/root_asset.js' => new MappedAsset('root_asset.js', publicPathWithoutDigest: '/assets/root_asset.js'),
307+
'/project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
308+
'/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
309+
'/project/root_asset.js' => new MappedAsset('root_asset.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/root_asset.js'),
310310
default => throw new \RuntimeException(sprintf('Unexpected source path "%s"', $path)),
311311
};
312312
});
@@ -320,9 +320,9 @@ public function testCompileFindsRelativePathsViaSourcePath()
320320
$compiler = new JavaScriptImportPathCompiler($this->createMock(ImportMapConfigReader::class));
321321
$compiler->compile($input, $inputAsset, $assetMapper);
322322
$this->assertCount(3, $inputAsset->getJavaScriptImports());
323-
$this->assertSame('other.js', $inputAsset->getJavaScriptImports()[0]->asset->logicalPath);
324-
$this->assertSame('subdir/foo.js', $inputAsset->getJavaScriptImports()[1]->asset->logicalPath);
325-
$this->assertSame('root_asset.js', $inputAsset->getJavaScriptImports()[2]->asset->logicalPath);
323+
$this->assertSame('other.js', $inputAsset->getJavaScriptImports()[0]->assetLogicalPath);
324+
$this->assertSame('subdir/foo.js', $inputAsset->getJavaScriptImports()[1]->assetLogicalPath);
325+
$this->assertSame('root_asset.js', $inputAsset->getJavaScriptImports()[2]->assetLogicalPath);
326326
}
327327

328328
public function testCompileFindsRelativePathsWithWindowsPathsViaSourcePath()
@@ -337,9 +337,9 @@ public function testCompileFindsRelativePathsWithWindowsPathsViaSourcePath()
337337
->method('getAssetFromSourcePath')
338338
->willReturnCallback(function ($path) {
339339
return match ($path) {
340-
'C://project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
341-
'C://project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
342-
'C://project/root_asset.js' => new MappedAsset('root_asset.js', publicPathWithoutDigest: '/assets/root_asset.js'),
340+
'C://project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
341+
'C://project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
342+
'C://project/root_asset.js' => new MappedAsset('root_asset.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/root_asset.js'),
343343
default => throw new \RuntimeException(sprintf('Unexpected source path "%s"', $path)),
344344
};
345345
});
@@ -366,7 +366,7 @@ public function testImportPathsCanUpdateForDifferentPublicPath(string $input, st
366366
$asset = new MappedAsset('app.js', '/path/to/assets/app.js', publicPathWithoutDigest: $inputAssetPublicPath);
367367

368368
$assetMapper = $this->createMock(AssetMapperInterface::class);
369-
$importedAsset = new MappedAsset('anything', publicPathWithoutDigest: $importedPublicPath);
369+
$importedAsset = new MappedAsset('anything', '/can/be/anything.js', publicPathWithoutDigest: $importedPublicPath);
370370
$assetMapper->expects($this->once())
371371
->method('getAssetFromSourcePath')
372372
->willReturn($importedAsset);
@@ -436,7 +436,7 @@ public function testCompileHandlesCircularRelativeAssets()
436436
$input = 'import "./other.js";';
437437
$compiler->compile($input, $appAsset, $assetMapper);
438438
$this->assertCount(1, $appAsset->getJavaScriptImports());
439-
$this->assertSame($otherAsset, $appAsset->getJavaScriptImports()[0]->asset);
439+
$this->assertSame($otherAsset->logicalPath, $appAsset->getJavaScriptImports()[0]->assetLogicalPath);
440440
}
441441

442442
public function testCompileHandlesCircularBareImportAssets()
@@ -464,7 +464,7 @@ public function testCompileHandlesCircularBareImportAssets()
464464
$input = 'import "@popperjs/core";';
465465
$compiler->compile($input, $bootstrapAsset, $assetMapper);
466466
$this->assertCount(1, $bootstrapAsset->getJavaScriptImports());
467-
$this->assertSame($popperAsset, $bootstrapAsset->getJavaScriptImports()[0]->asset);
467+
$this->assertSame($popperAsset->logicalPath, $bootstrapAsset->getJavaScriptImports()[0]->assetLogicalPath);
468468
}
469469

470470
/**
@@ -490,7 +490,7 @@ public function testMissingImportMode(string $sourceLogicalName, string $input,
490490
->method('getAssetFromSourcePath')
491491
->willReturnCallback(function ($sourcePath) {
492492
return match ($sourcePath) {
493-
'/path/to/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
493+
'/path/to/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
494494
default => null,
495495
};
496496
}

‎src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public function testAssetConfigCacheResourceContainsDependencies()
9494
$deeplyNestedAsset = new MappedAsset('file4.js', realpath(__DIR__.'/../Fixtures/dir2/file4.js'));
9595

9696
$file6Asset = new MappedAsset('file6.js', realpath(__DIR__.'/../Fixtures/dir2/subdir/file6.js'));
97-
$deeplyNestedAsset->addJavaScriptImport(new JavaScriptImport('file6', asset: $file6Asset));
97+
$deeplyNestedAsset->addJavaScriptImport(new JavaScriptImport('file6', assetLogicalPath: $file6Asset->logicalPath, assetSourcePath: $file6Asset->sourcePath));
9898

9999
$dependentOnContentAsset->addDependency($deeplyNestedAsset);
100100
$mappedAsset->addDependency($dependentOnContentAsset);

0 commit comments

Comments
 (0)
0