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

Author: nicolas-grekas

Compiler/JavaScriptImportPathCompiler.php

@@ -72,7 +72,8 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
             $addToImportMap = $isRelativeImport;
             $asset->addJavaScriptImport(new JavaScriptImport(
                 $addToImportMap ? $dependentAsset->publicPathWithoutDigest : $importedModule,
-                $dependentAsset,
+                $dependentAsset->logicalPath,
+                $dependentAsset->sourcePath,
                 $isLazy,
                 $addToImportMap,
             ));

Factory/CachedMappedAssetFactory.php

@@ -69,7 +69,7 @@ private function collectResourcesFromAsset(MappedAsset $mappedAsset): array
         }
 
         foreach ($mappedAsset->getJavaScriptImports() as $import) {
-            $resources[] = new FileExistenceResource($import->asset->sourcePath);
+            $resources[] = new FileExistenceResource($import->assetSourcePath);
         }
 
         return $resources;

ImportMap/ImportMapGenerator.php

@@ -179,10 +179,15 @@ private function addImplicitEntries(ImportMapEntry $entry, array $currentImportE
 
             // check if this import requires an automatic importmap entry
             if ($javaScriptImport->addImplicitlyToImportMap) {
+                if (!$importedAsset = $this->assetMapper->getAsset($javaScriptImport->assetLogicalPath)) {
+                    // should not happen at this point, unless something added a bogus JavaScriptImport to this asset
+                    throw new LogicException(sprintf('Cannot find imported JavaScript asset "%s" in asset mapper.', $javaScriptImport->assetLogicalPath));
+                }
+
                 $nextEntry = ImportMapEntry::createLocal(
                     $importName,
-                    ImportMapType::tryFrom($javaScriptImport->asset->publicExtension) ?: ImportMapType::JS,
-                    $javaScriptImport->asset->logicalPath,
+                    ImportMapType::tryFrom($importedAsset->publicExtension) ?: ImportMapType::JS,
+                    $importedAsset->logicalPath,
                     false,
                 );
 
@@ -223,7 +228,11 @@ private function findEagerImports(MappedAsset $asset): array
             $dependencies[] = $javaScriptImport->importName;
 
             // Follow its imports!
-            $dependencies = array_merge($dependencies, $this->findEagerImports($javaScriptImport->asset));
+            if (!$nextAsset = $this->assetMapper->getAsset($javaScriptImport->assetLogicalPath)) {
+                // should not happen at this point, unless something added a bogus JavaScriptImport to this asset
+                throw new LogicException(sprintf('Cannot find imported JavaScript asset "%s" in asset mapper.', $javaScriptImport->assetLogicalPath));
+            }
+            $dependencies = array_merge($dependencies, $this->findEagerImports($nextAsset));
         }
 
         return $dependencies;
@@ -232,7 +241,11 @@ private function findEagerImports(MappedAsset $asset): array
     private function createMissingImportMapAssetException(ImportMapEntry $entry): \InvalidArgumentException
     {
         if ($entry->isRemotePackage()) {
-            throw new LogicException(sprintf('The "%s" vendor asset is missing. Try running the "importmap:install" command.', $entry->importName));
+            if (!is_file($entry->path)) {
+                throw new LogicException(sprintf('The "%s" vendor asset is missing. Try running the "importmap:install" command.', $entry->importName));
+            }
+
+            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));
         }
 
         throw new LogicException(sprintf('The asset "%s" cannot be found in any asset map paths.', $entry->path));

ImportMap/ImportMapRenderer.php

@@ -62,6 +62,11 @@ public function render(string|array $entryPoint, array $attributes = []): string
                 continue;
             }
 
+            // for subdirectories or CDNs, the import name needs to be the full URL
+            if (str_starts_with($importName, '/') && $this->assetPackages) {
+                $importName = $this->assetPackages->getUrl(ltrim($importName, '/'));
+            }
+
             $preload = $data['preload'] ?? false;
             if ('css' !== $data['type']) {
                 $importMap[$importName] = $path;

ImportMap/JavaScriptImport.php

@@ -11,21 +11,20 @@
 
 namespace Symfony\Component\AssetMapper\ImportMap;
 
-use Symfony\Component\AssetMapper\MappedAsset;
-
 /**
  * Represents a module that was imported by a JavaScript file.
  */
 final class JavaScriptImport
 {
     /**
-     * @param string      $importName               The name of the import needed in the importmap, e.g. "/foo.js" or "react"
-     * @param MappedAsset $asset                    The asset that was imported
-     * @param bool        $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
+     * @param string $importName               The name of the import needed in the importmap, e.g. "/foo.js" or "react"
+     * @param string $assetLogicalPath         Logical path to the mapped ass that was imported
+     * @param bool   $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
      */
     public function __construct(
         public readonly string $importName,
-        public readonly MappedAsset $asset,
+        public readonly string $assetLogicalPath,
+        public readonly string $assetSourcePath,
         public readonly bool $isLazy = false,
         public bool $addImplicitlyToImportMap = false,
     ) {

ImportMap/Resolver/JsDelivrEsmResolver.php

@@ -26,7 +26,7 @@ final class JsDelivrEsmResolver implements PackageResolverInterface
     public const URL_PATTERN_DIST = self::URL_PATTERN_DIST_CSS.'/+esm';
     public const URL_PATTERN_ENTRYPOINT = 'https://data.jsdelivr.com/v1/packages/npm/%s@%s/entrypoints';
 
-    public const IMPORT_REGEX = '{from"/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm"}';
+    public const IMPORT_REGEX = '#(?:import\s*(?:(?:\{[^}]*\}|\w+|\*\s*as\s+\w+)\s*\bfrom\s*)?|export\s*(?:\{[^}]*\}|\*)\s*from\s*)("/npm/((?:@[^/]+/)?[^@]+?)(?:@([^/]+))?((?:/[^/]+)*?)/\+esm")#';
 
     private HttpClientInterface $httpClient;
 
@@ -129,8 +129,9 @@ public function resolvePackages(array $packagesToRequire): array
 
             $entrypoints = $cssEntrypointResponse->toArray()['entrypoints'] ?? [];
             $cssFile = $entrypoints['css']['file'] ?? null;
+            $guessed = $entrypoints['css']['guessed'] ?? true;
 
-            if (!$cssFile) {
+            if (!$cssFile || $guessed) {
                 continue;
             }
 
@@ -221,9 +222,9 @@ private function fetchPackageRequirementsFromImports(string $content): array
         // imports from jsdelivr follow a predictable format
         preg_match_all(self::IMPORT_REGEX, $content, $matches);
         $dependencies = [];
-        foreach ($matches[1] as $index => $packageName) {
-            $version = $matches[2][$index] ?: null;
-            $packageName .= $matches[3][$index]; // add the path if any
+        foreach ($matches[2] as $index => $packageName) {
+            $version = $matches[3][$index] ?: null;
+            $packageName .= $matches[4][$index]; // add the path if any
 
             $dependencies[] = new PackageRequireOptions($packageName, $version);
         }
@@ -239,10 +240,11 @@ private function fetchPackageRequirementsFromImports(string $content): array
     private function makeImportsBare(string $content, array &$dependencies): string
     {
         $content = preg_replace_callback(self::IMPORT_REGEX, function ($matches) use (&$dependencies) {
-            $packageName = $matches[1].$matches[3]; // add the path if any
+            $packageName = $matches[2].$matches[4]; // add the path if any
             $dependencies[] = $packageName;
 
-            return sprintf('from"%s"', $packageName);
+            // replace the "/npm/package@version/+esm" with "package@version"
+            return str_replace($matches[1], sprintf('"%s"', $packageName), $matches[0]);
         }, $content);
 
         // source maps are not also downloaded - so remove the sourceMappingURL

MappedAsset.php

@@ -94,6 +94,8 @@ public function __construct(
     }
 
     /**
+     * Assets that the content of this asset depends on - for internal caching.
+     *
      * @return MappedAsset[]
      */
     public function getDependencies(): array

Tests/Compiler/JavaScriptImportPathCompilerTest.php

@@ -58,7 +58,7 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
             ->method('getAsset')
             ->willReturnCallback(function ($path) {
                 return match ($path) {
-                    'module_in_importmap_local_asset.js' => new MappedAsset('module_in_importmap_local_asset.js', publicPathWithoutDigest: '/assets/module_in_importmap_local_asset.js'),
+                    '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'),
                     default => null,
                 };
             });
@@ -67,11 +67,11 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
             ->method('getAssetFromSourcePath')
             ->willReturnCallback(function ($path) {
                 return match ($path) {
-                    '/project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
-                    '/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
-                    '/project/assets/styles.css' => new MappedAsset('styles.css', publicPathWithoutDigest: '/assets/styles.css'),
-                    '/project/assets/vendor/module_in_importmap_remote.js' => new MappedAsset('module_in_importmap_remote.js', publicPathWithoutDigest: '/assets/module_in_importmap_remote.js'),
-                    '/project/assets/vendor/@popperjs/core.js' => new MappedAsset('assets/vendor/@popperjs/core.js', publicPathWithoutDigest: '/assets/@popperjs/core.js'),
+                    '/project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
+                    '/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
+                    '/project/assets/styles.css' => new MappedAsset('styles.css', '/can/be/anything.js', publicPathWithoutDigest: '/assets/styles.css'),
+                    '/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'),
+                    '/project/assets/vendor/@popperjs/core.js' => new MappedAsset('assets/vendor/@popperjs/core.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/@popperjs/core.js'),
                     default => null,
                 };
             });
@@ -81,7 +81,7 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
         $this->assertSame($input, $compiler->compile($input, $asset, $assetMapper));
         $actualImports = [];
         foreach ($asset->getJavaScriptImports() as $import) {
-            $actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->asset->logicalPath, 'add' => $import->addImplicitlyToImportMap];
+            $actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->assetLogicalPath, 'add' => $import->addImplicitlyToImportMap];
         }
 
         $this->assertEquals($expectedJavaScriptImports, $actualImports);
@@ -304,9 +304,9 @@ public function testCompileFindsRelativePathsViaSourcePath()
             ->method('getAssetFromSourcePath')
             ->willReturnCallback(function ($path) {
                 return match ($path) {
-                    '/project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
-                    '/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
-                    '/project/root_asset.js' => new MappedAsset('root_asset.js', publicPathWithoutDigest: '/assets/root_asset.js'),
+                    '/project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
+                    '/project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
+                    '/project/root_asset.js' => new MappedAsset('root_asset.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/root_asset.js'),
                     default => throw new \RuntimeException(sprintf('Unexpected source path "%s"', $path)),
                 };
             });
@@ -320,9 +320,9 @@ public function testCompileFindsRelativePathsViaSourcePath()
         $compiler = new JavaScriptImportPathCompiler($this->createMock(ImportMapConfigReader::class));
         $compiler->compile($input, $inputAsset, $assetMapper);
         $this->assertCount(3, $inputAsset->getJavaScriptImports());
-        $this->assertSame('other.js', $inputAsset->getJavaScriptImports()[0]->asset->logicalPath);
-        $this->assertSame('subdir/foo.js', $inputAsset->getJavaScriptImports()[1]->asset->logicalPath);
-        $this->assertSame('root_asset.js', $inputAsset->getJavaScriptImports()[2]->asset->logicalPath);
+        $this->assertSame('other.js', $inputAsset->getJavaScriptImports()[0]->assetLogicalPath);
+        $this->assertSame('subdir/foo.js', $inputAsset->getJavaScriptImports()[1]->assetLogicalPath);
+        $this->assertSame('root_asset.js', $inputAsset->getJavaScriptImports()[2]->assetLogicalPath);
     }
 
     public function testCompileFindsRelativePathsWithWindowsPathsViaSourcePath()
@@ -337,9 +337,9 @@ public function testCompileFindsRelativePathsWithWindowsPathsViaSourcePath()
             ->method('getAssetFromSourcePath')
             ->willReturnCallback(function ($path) {
                 return match ($path) {
-                    'C://project/assets/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
-                    'C://project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
-                    'C://project/root_asset.js' => new MappedAsset('root_asset.js', publicPathWithoutDigest: '/assets/root_asset.js'),
+                    'C://project/assets/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
+                    'C://project/assets/subdir/foo.js' => new MappedAsset('subdir/foo.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/subdir/foo.js'),
+                    'C://project/root_asset.js' => new MappedAsset('root_asset.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/root_asset.js'),
                     default => throw new \RuntimeException(sprintf('Unexpected source path "%s"', $path)),
                 };
             });
@@ -366,7 +366,7 @@ public function testImportPathsCanUpdateForDifferentPublicPath(string $input, st
         $asset = new MappedAsset('app.js', '/path/to/assets/app.js', publicPathWithoutDigest: $inputAssetPublicPath);
 
         $assetMapper = $this->createMock(AssetMapperInterface::class);
-        $importedAsset = new MappedAsset('anything', publicPathWithoutDigest: $importedPublicPath);
+        $importedAsset = new MappedAsset('anything', '/can/be/anything.js', publicPathWithoutDigest: $importedPublicPath);
         $assetMapper->expects($this->once())
             ->method('getAssetFromSourcePath')
             ->willReturn($importedAsset);
@@ -436,7 +436,7 @@ public function testCompileHandlesCircularRelativeAssets()
         $input = 'import "./other.js";';
         $compiler->compile($input, $appAsset, $assetMapper);
         $this->assertCount(1, $appAsset->getJavaScriptImports());
-        $this->assertSame($otherAsset, $appAsset->getJavaScriptImports()[0]->asset);
+        $this->assertSame($otherAsset->logicalPath, $appAsset->getJavaScriptImports()[0]->assetLogicalPath);
     }
 
     public function testCompileHandlesCircularBareImportAssets()
@@ -464,7 +464,7 @@ public function testCompileHandlesCircularBareImportAssets()
         $input = 'import "@popperjs/core";';
         $compiler->compile($input, $bootstrapAsset, $assetMapper);
         $this->assertCount(1, $bootstrapAsset->getJavaScriptImports());
-        $this->assertSame($popperAsset, $bootstrapAsset->getJavaScriptImports()[0]->asset);
+        $this->assertSame($popperAsset->logicalPath, $bootstrapAsset->getJavaScriptImports()[0]->assetLogicalPath);
     }
 
     /**
@@ -490,7 +490,7 @@ public function testMissingImportMode(string $sourceLogicalName, string $input,
             ->method('getAssetFromSourcePath')
             ->willReturnCallback(function ($sourcePath) {
                 return match ($sourcePath) {
-                    '/path/to/other.js' => new MappedAsset('other.js', publicPathWithoutDigest: '/assets/other.js'),
+                    '/path/to/other.js' => new MappedAsset('other.js', '/can/be/anything.js', publicPathWithoutDigest: '/assets/other.js'),
                     default => null,
                 };
             }

Tests/Factory/CachedMappedAssetFactoryTest.php

@@ -94,7 +94,7 @@ public function testAssetConfigCacheResourceContainsDependencies()
         $deeplyNestedAsset = new MappedAsset('file4.js', realpath(__DIR__.'/../Fixtures/dir2/file4.js'));
 
         $file6Asset = new MappedAsset('file6.js', realpath(__DIR__.'/../Fixtures/dir2/subdir/file6.js'));
-        $deeplyNestedAsset->addJavaScriptImport(new JavaScriptImport('file6', asset: $file6Asset));
+        $deeplyNestedAsset->addJavaScriptImport(new JavaScriptImport('file6', assetLogicalPath: $file6Asset->logicalPath, assetSourcePath: $file6Asset->sourcePath));
 
         $dependentOnContentAsset->addDependency($deeplyNestedAsset);
         $mappedAsset->addDependency($dependentOnContentAsset);