[AssetMapper] Allowing circular references in JavaScriptImportPathCompiler

Author: weaverryan

Compiler/JavaScriptImportPathCompiler.php

@@ -148,11 +148,15 @@ private function findAssetForBareImport(string $importedModule, AssetMapperInter
             return null;
         }
 
-        if ($asset = $assetMapper->getAsset($importMapEntry->path)) {
-            return $asset;
-        }
+        try {
+            if ($asset = $assetMapper->getAsset($importMapEntry->path)) {
+                return $asset;
+            }
 
-        return $assetMapper->getAssetFromSourcePath($importMapEntry->path);
+            return $assetMapper->getAssetFromSourcePath($importMapEntry->path);
+        } catch (CircularAssetsException $exception) {
+            return $exception->getIncompleteMappedAsset();
+        }
     }
 
     private function findAssetForRelativeImport(string $importedModule, MappedAsset $asset, AssetMapperInterface $assetMapper): ?MappedAsset
@@ -168,8 +172,7 @@ private function findAssetForRelativeImport(string $importedModule, MappedAsset
             return null;
         }
 
-        $dependentAsset = $assetMapper->getAssetFromSourcePath($resolvedSourcePath);
-
+        $dependentAsset = $assetMapper->getAsset($resolvedSourcePath);
         if ($dependentAsset) {
             return $dependentAsset;
         }

Exception/CircularAssetsException.php

@@ -11,9 +11,26 @@
 
 namespace Symfony\Component\AssetMapper\Exception;
 
+use Symfony\Component\AssetMapper\MappedAsset;
+
 /**
  * Thrown when a circular reference is detected while creating an asset.
  */
 class CircularAssetsException extends RuntimeException
 {
+    public function __construct(private MappedAsset $mappedAsset, string $message = '', int $code = 0, \Throwable $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+    }
+
+    /**
+     * Returns the asset that was being created when the circular reference was detected.
+     *
+     * This asset will not be fully initialized: it will be missing some
+     * properties like digest and content.
+     */
+    public function getIncompleteMappedAsset(): MappedAsset
+    {
+        return $this->mappedAsset;
+    }
 }

Factory/MappedAssetFactory.php

@@ -37,13 +37,14 @@ public function __construct(
     public function createMappedAsset(string $logicalPath, string $sourcePath): ?MappedAsset
     {
         if (isset($this->assetsBeingCreated[$logicalPath])) {
-            throw new CircularAssetsException(sprintf('Circular reference detected while creating asset for "%s": "%s".', $logicalPath, implode(' -> ', $this->assetsBeingCreated).' -> '.$logicalPath));
+            throw new CircularAssetsException($this->assetsCache[$logicalPath], sprintf('Circular reference detected while creating asset for "%s": "%s".', $logicalPath, implode(' -> ', $this->assetsBeingCreated).' -> '.$logicalPath));
         }
         $this->assetsBeingCreated[$logicalPath] = $logicalPath;
 
         if (!isset($this->assetsCache[$logicalPath])) {
             $isVendor = $this->isVendor($sourcePath);
             $asset = new MappedAsset($logicalPath, $sourcePath, $this->assetsPathResolver->resolvePublicPath($logicalPath), isVendor: $isVendor);
+            $this->assetsCache[$logicalPath] = $asset;
 
             $content = $this->compileContent($asset);
             [$digest, $isPredigested] = $this->getDigest($asset, $content);

Tests/Compiler/JavaScriptImportPathCompilerTest.php

@@ -417,6 +417,48 @@ public static function providePathsCanUpdateTests(): iterable
         ];
     }
 
+    public function testCompileHandlesCircularRelativeAssets()
+    {
+        $appAsset = new MappedAsset('app.js', 'anythingapp', '/assets/app.js');
+        $otherAsset = new MappedAsset('other.js', 'anythingother', '/assets/other.js');
+
+        $importMapConfigReader = $this->createMock(ImportMapConfigReader::class);
+        $assetMapper = $this->createMock(AssetMapperInterface::class);
+        $assetMapper->expects($this->once())
+            ->method('getAsset')
+            ->with('other.js')
+            ->willThrowException(new CircularAssetsException($otherAsset));
+
+        $compiler = new JavaScriptImportPathCompiler($importMapConfigReader);
+        $input = 'import "./other.js";';
+        $compiler->compile($input, $appAsset, $assetMapper);
+        $this->assertCount(1, $appAsset->getJavaScriptImports());
+        $this->assertSame($otherAsset, $appAsset->getJavaScriptImports()[0]->asset);
+    }
+
+    public function testCompileHandlesCircularBareImportAssets()
+    {
+        $bootstrapAsset = new MappedAsset('bootstrap', 'anythingbootstrap', '/assets/bootstrap.js');
+        $popperAsset = new MappedAsset('@popperjs/core', 'anythingpopper', '/assets/popper.js');
+
+        $importMapConfigReader = $this->createMock(ImportMapConfigReader::class);
+        $importMapConfigReader->expects($this->once())
+            ->method('findRootImportMapEntry')
+            ->with('@popperjs/core')
+            ->willReturn(ImportMapEntry::createRemote('@popperjs/core', ImportMapType::JS, '/path/to/vendor/@popperjs/core.js', '1.2.3', 'could_be_anything', false));
+        $assetMapper = $this->createMock(AssetMapperInterface::class);
+        $assetMapper->expects($this->once())
+            ->method('getAssetFromSourcePath')
+            ->with('/path/to/vendor/@popperjs/core.js')
+            ->willThrowException(new CircularAssetsException($popperAsset));
+
+        $compiler = new JavaScriptImportPathCompiler($importMapConfigReader);
+        $input = 'import "@popperjs/core";';
+        $compiler->compile($input, $bootstrapAsset, $assetMapper);
+        $this->assertCount(1, $bootstrapAsset->getJavaScriptImports());
+        $this->assertSame($popperAsset, $bootstrapAsset->getJavaScriptImports()[0]->asset);
+    }
+
     /**
      * @dataProvider provideMissingImportModeTests
      */
@@ -487,7 +529,7 @@ public function testErrorMessageAvoidsCircularException()
                 }
 
                 if ('htmx.js' === $logicalPath) {
-                    throw new CircularAssetsException();
+                    throw new CircularAssetsException(new MappedAsset('htmx.js'));
                 }
             });
 

Tests/Factory/MappedAssetFactoryTest.php

@@ -18,7 +18,7 @@
 use Symfony\Component\AssetMapper\Compiler\AssetCompilerInterface;
 use Symfony\Component\AssetMapper\Compiler\CssAssetUrlCompiler;
 use Symfony\Component\AssetMapper\Compiler\JavaScriptImportPathCompiler;
-use Symfony\Component\AssetMapper\Exception\RuntimeException;
+use Symfony\Component\AssetMapper\Exception\CircularAssetsException;
 use Symfony\Component\AssetMapper\Factory\MappedAssetFactory;
 use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader;
 use Symfony\Component\AssetMapper\MappedAsset;
@@ -85,7 +85,7 @@ public function testCreateMappedAssetWithContentErrorsOnCircularReferences()
     {
         $factory = $this->createFactory();
 
-        $this->expectException(RuntimeException::class);
+        $this->expectException(CircularAssetsException::class);
         $this->expectExceptionMessage('Circular reference detected while creating asset for "circular1.css": "circular1.css -> circular2.css -> circular1.css".');
         $factory->createMappedAsset('circular1.css', __DIR__.'/../Fixtures/circular_dir/circular1.css');
     }