[AssetMapper] Allow simple, relative paths in importmap.php

symfony · nicolas-grekas · Sep 29, 2023 · Sep 23, 2023 · Sep 29, 2023 · 978b14d51d55cc53dd74cdee356cc83b24f2fdc5
commit 978b14d51d55cc53dd74cdee356cc83b24f2fdc5
diff --git a/src/Symfony/Component/AssetMapper/CHANGELOG.md b/src/Symfony/Component/AssetMapper/CHANGELOG.md
@@ -7,6 +7,7 @@ CHANGELOG
  * Mark the component as non experimental
  * Add CSS support to the importmap
  * Add "entrypoints" concept to the importmap
+ * Allow relative path strings in the importmap
  * Add `PreAssetsCompileEvent` event when running `asset-map:compile`
  * Add support for importmap paths to use the Asset component (for subdirectories)
  * Removed the `importmap:export` command

diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapConfigReader.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapConfigReader.php
@@ -107,4 +107,9 @@ public function writeEntries(ImportMapEntries $entries): void
 
         EOF);
     }
+
+    public function getRootDirectory(): string
+    {
+        return \dirname($this->importMapConfigPath);
+    }
 }
diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapEntry.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapEntry.php
@@ -19,10 +19,10 @@
 final class ImportMapEntry
 {
     public function __construct(
+        public readonly string $importName,
         /**
-         * The logical path to this asset if local or downloaded.
+         * The path to the asset if local or downloaded.
          */
-        public readonly string $importName,
         public readonly ?string $path = null,
         public readonly ?string $url = null,
         public readonly bool $isDownloaded = false,

diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php
@@ -108,7 +108,7 @@ public function downloadMissingPackages(): array
         $downloadedPackages = [];
 
         foreach ($entries as $entry) {
-            if (!$entry->isDownloaded || $this->assetMapper->getAsset($entry->path)) {
+            if (!$entry->isDownloaded || $this->findAsset($entry->path)) {
                 continue;
             }
 
@@ -211,7 +211,7 @@ public function getRawImportMapData(): array
         $rawImportMapData = [];
         foreach ($allEntries as $entry) {
             if ($entry->path) {
-                $asset = $this->assetMapper->getAsset($entry->path);
+                $asset = $this->findAsset($entry->path);
 
                 if (!$asset) {
                     if ($entry->isDownloaded) {
@@ -330,11 +330,15 @@ private function requirePackages(array $packagesToRequire, ImportMapEntries $imp
             }
 
             $path = $requireOptions->path;
-            if (is_file($path)) {
-                $path = $this->assetMapper->getAssetFromSourcePath($path)?->logicalPath;
-                if (null === $path) {
-                    throw new \LogicException(sprintf('The path "%s" of the package "%s" cannot be found in any asset map paths.', $requireOptions->path, $requireOptions->packageName));
-                }
+            if (!$asset = $this->findAsset($path)) {
+                throw new \LogicException(sprintf('The path "%s" of the package "%s" cannot be found: either pass the logical name of the asset or a relative path starting with "./".', $requireOptions->path, $requireOptions->packageName));
+            }
+
+            $rootImportMapDir = $this->importMapConfigReader->getRootDirectory();
+            // convert to a relative path (or fallback to the logical path)
+            $path = $asset->logicalPath;
+            if ($rootImportMapDir && str_starts_with(realpath($asset->sourcePath), realpath($rootImportMapDir))) {
+                $path = './'.substr(realpath($asset->sourcePath), \strlen(realpath($rootImportMapDir)) + 1);
             }
 
             $newEntry = new ImportMapEntry(
@@ -384,7 +388,7 @@ private function cleanupPackageFiles(ImportMapEntry $entry): void
             return;
         }
 
-        $asset = $this->assetMapper->getAsset($entry->path);
+        $asset = $this->findAsset($entry->path);
 
         if (!$asset) {
             throw new \LogicException(sprintf('The path "%s" of the package "%s" cannot be found in any asset map paths.', $entry->path, $entry->importName));
@@ -418,7 +422,7 @@ private function addImplicitEntries(ImportMapEntry $entry, array $currentImportE
             return $currentImportEntries;
         }
 
-        if (!$asset = $this->assetMapper->getAsset($entry->path)) {
+        if (!$asset = $this->findAsset($entry->path)) {
             // should only be possible at this point for root importmap.php entries
             throw new \InvalidArgumentException(sprintf('The asset "%s" mentioned in "importmap.php" cannot be found in any asset map paths.', $entry->path));
         }
@@ -498,7 +502,7 @@ private function findEagerEntrypointImports(string $entryName): array
             throw new \InvalidArgumentException(sprintf('The entrypoint "%s" is a remote package and cannot be used as an entrypoint.', $entryName));
         }
 
-        $asset = $this->assetMapper->getAsset($rootImportEntries->get($entryName)->path);
+        $asset = $this->findAsset($rootImportEntries->get($entryName)->path);
         if (!$asset) {
             throw new \InvalidArgumentException(sprintf('The path "%s" of the entrypoint "%s" mentioned in "importmap.php" cannot be found in any asset map paths.', $rootImportEntries->get($entryName)->path, $entryName));
         }
@@ -529,4 +533,20 @@ private static function getImportMapTypeFromFilename(string $path): ImportMapTyp
     {
         return str_ends_with($path, '.css') ? ImportMapType::CSS : ImportMapType::JS;
     }
+
+    /**
+     * Finds the MappedAsset allowing for a "logical path", relative or absolute filesystem path.
+     */
+    private function findAsset(string $path): ?MappedAsset
+    {
+        if ($asset = $this->assetMapper->getAsset($path)) {
+            return $asset;
+        }
+
+        if (str_starts_with($path, '.')) {
+            $path = $this->importMapConfigReader->getRootDirectory().'/'.$path;
+        }
+
+        return $this->assetMapper->getAssetFromSourcePath($path);
+    }
 }
diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapConfigReaderTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapConfigReaderTest.php
@@ -102,4 +102,10 @@ public function testGetEntriesAndWriteEntries()
 
         $this->assertSame($originalImportMapData, $newImportMapData);
     }
+
+    public function testGetRootDirectory()
+    {
+        $configReader = new ImportMapConfigReader(__DIR__.'/../fixtures/importmap.php');
+        $this->assertSame(__DIR__.'/../fixtures', $configReader->getRootDirectory());
+    }
 }
diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapManagerTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapManagerTest.php
@@ -65,6 +65,9 @@ public function testGetRawImportMapData(array $importMapEntries, array $mappedAs
         $manager = $this->createImportMapManager();
         $this->mockImportMap($importMapEntries);
         $this->mockAssetMapper($mappedAssets);
+        $this->configReader->expects($this->any())
+            ->method('getRootDirectory')
+            ->willReturn('/fake/root');
 
         $this->assertEquals($expectedData, $manager->getRawImportMapData());
     }
@@ -255,7 +258,7 @@ public function getRawImportMapDataTests(): iterable
                 'imports_simple' => [
                     'path' => '/assets/imports_simple-d1g3st.js',
                     'type' => 'js',
-                ]
+                ],
             ],
         ];
 
@@ -304,6 +307,51 @@ public function getRawImportMapDataTests(): iterable
                 ],
             ],
         ];
+
+        yield 'it handles a relative path file' => [
+            [
+                new ImportMapEntry(
+                    'app',
+                    path: './assets/app.js',
+                ),
+            ],
+            [
+                new MappedAsset(
+                    'app.js',
+                    // /fake/root is the mocked root directory
+                    '/fake/root/assets/app.js',
+                    publicPath: '/assets/app.js',
+                ),
+            ],
+            [
+                'app' => [
+                    'path' => '/assets/app.js',
+                    'type' => 'js',
+                ],
+            ],
+        ];
+
+        yield 'it handles an absolute path file' => [
+            [
+                new ImportMapEntry(
+                    'app',
+                    path: '/some/path/assets/app.js',
+                ),
+            ],
+            [
+                new MappedAsset(
+                    'app.js',
+                    '/some/path/assets/app.js',
+                    publicPath: '/assets/app.js',
+                ),
+            ],
+            [
+                'app' => [
+                    'path' => '/assets/app.js',
+                    'type' => 'js',
+                ],
+            ],
+        ];
     }
 
     public function testGetRawImportDataUsesCacheFile()
@@ -609,19 +657,22 @@ public function testRequire(array $packages, int $expectedProviderPackageArgumen
                 foreach ($expectedDownloadedFiles as $file => $contents) {
                     $expectedPath = self::$writableRoot.'/assets/vendor/'.$file;
                     if (realpath($expectedPath) === realpath($sourcePath)) {
-                        return new MappedAsset('vendor/'.$file);
+                        return new MappedAsset('vendor/'.$file, $sourcePath);
                     }
                 }
 
                 if (str_ends_with($sourcePath, 'some_file.js')) {
                     // physical file we point to in one test
-                    return new MappedAsset('some_file.js');
+                    return new MappedAsset('some_file.js', $sourcePath);
                 }
 
                 return null;
             })
         ;
 
+        $this->configReader->expects($this->any())
+            ->method('getRootDirectory')
+            ->willReturn(self::$writableRoot);
         $this->configReader->expects($this->once())
             ->method('getEntries')
             ->willReturn(new ImportMapEntries())
@@ -789,7 +840,8 @@ public static function getRequirePackageTests(): iterable
             'resolvedPackages' => [],
             'expectedImportMap' => [
                 'some/module' => [
-                    'path' => 'some_file.js',
+                    // converted to relative path
+                    'path' => './assets/some_file.js',
                 ],
             ],
             'expectedDownloadedFiles' => [],
@@ -849,7 +901,7 @@ public function testUpdateAll()
 
         $this->mockAssetMapper([
             new MappedAsset('vendor/moo.js', self::$writableRoot.'/assets/vendor/moo.js'),
-        ]);
+        ], false);
         $this->assetMapper->expects($this->any())
             ->method('getAssetFromSourcePath')
             ->willReturnCallback(function (string $sourcePath) {
@@ -932,6 +984,9 @@ public function testUpdateWithSpecificPackages()
             ])
         ;
 
+        $this->configReader->expects($this->any())
+            ->method('getRootDirectory')
+            ->willReturn(self::$writableRoot);
         $this->configReader->expects($this->once())
             ->method('writeEntries')
             ->with($this->callback(function (ImportMapEntries $entries) {
@@ -947,10 +1002,6 @@ public function testUpdateWithSpecificPackages()
         $this->mockAssetMapper([
             new MappedAsset('vendor/cowsay.js', self::$writableRoot.'/assets/vendor/cowsay.js'),
         ]);
-        $this->assetMapper->expects($this->once())
-            ->method('getAssetFromSourcePath')
-            ->willReturn(new MappedAsset('vendor/cowsay.js'))
-        ;
 
         $manager->update(['cowsay']);
         $actualContents = file_get_contents(self::$writableRoot.'/assets/vendor/cowsay.js');
@@ -968,13 +1019,14 @@ public function testDownloadMissingPackages()
         $this->mockAssetMapper([
             // fake that vendor/lodash.js exists, but not stimulus
             new MappedAsset('vendor/lodash.js'),
-        ]);
-        $this->assetMapper->expects($this->once())
+        ], false);
+        $this->assetMapper->expects($this->any())
             ->method('getAssetFromSourcePath')
-            ->with($this->cal
E917
lback(function (string $sourcePath) {
-                return str_ends_with($sourcePath, 'assets/vendor/@hotwired/stimulus.js');
-            }))
-            ->willReturn(new MappedAsset('vendor/@hotwired/stimulus.js'))
+            ->willReturnCallback(function (string $sourcePath) {
+                if (str_ends_with($sourcePath, 'assets/vendor/@hotwired/stimulus.js')) {
+                    return new MappedAsset('vendor/@hotwired/stimulus.js');
+                }
+            })
         ;
 
         $response = $this->createMock(ResponseInterface::class);
@@ -1125,7 +1177,7 @@ private function mockImportMap(array $importMapEntries): void
     /**
      * @param MappedAsset[] $mappedAssets
      */
-    private function mockAssetMapper(array $mappedAssets): void
+    private function mockAssetMapper(array $mappedAssets, bool $mockGetAssetFromSourcePath = true): void
     {
         $this->assetMapper->expects($this->any())
             ->method('getAsset')
@@ -1139,6 +1191,44 @@ private function mockAssetMapper(array $mappedAssets): void
                 return null;
             })
         ;
+
+        if (!$mockGetAssetFromSourcePath) {
+            return;
+        }
+
+        $this->assetMapper->expects($this->any())
+            ->method('getAssetFromSourcePath')
+            ->willReturnCallback(function (string $sourcePath) use ($mappedAssets) {
+                // collapse ../ in paths and ./ in paths to mimic the realpath AssetMapper uses
+                $unCollapsePath = function (string $path) {
+                    $parts = explode('/', $path);
+                    $newParts = [];
+                    foreach ($parts as $part) {
+                        if ('..' === $part) {
+                            array_pop($newParts);
+
+                            continue;
+                        }
+
+                        if ('.' !== $part) {
+                            $newParts[] = $part;
+                        }
+                    }
+
+                    return implode('/', $newParts);
+                };
+
+                $sourcePath = $unCollapsePath($sourcePath);
+
+                foreach ($mappedAssets as $asset) {
+                    if (isset($asset->sourcePath) && $unCollapsePath($asset->sourcePath) === $sourcePath) {
+                        return $asset;
+                    }
+                }
+
+                return null;
+            })
+        ;
     }
 
     private function writeFile(string $filename, string $content): void
-Original file line number
+Diff line change
@@ Expand Up @@
             EOF);
         }
+        public function getRootDirectory(): string
+        {
+            return \dirname($this->importMapConfigPath);
+        }
     }