symfony · nicolas-grekas · Oct 31, 2023 · Oct 30, 2023
diff --git a/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php b/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
@@ -153,7 +153,7 @@ private function findAssetForBareImport(string $importedModule, AssetMapperInter
                 return $asset;
             }
 
-            return $assetMapper->getAssetFromSourcePath($importMapEntry->path);
+            return $assetMapper->getAssetFromSourcePath($this->importMapConfigReader->convertPathToFilesystemPath($importMapEntry->path));
         } catch (CircularAssetsException $exception) {
             return $exception->getIncompleteMappedAsset();
         }

diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapConfigReader.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapConfigReader.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\AssetMapper\ImportMap;
 
 use Symfony\Component\AssetMapper\Exception\RuntimeException;
+use Symfony\Component\Filesystem\Path;
 use Symfony\Component\VarExporter\VarExporter;
 
 /**
@@ -144,7 +145,40 @@ public function createRemoteEntry(string $importName, ImportMapType $type, strin
         return ImportMapEntry::createRemote($importName, $type, $path, $version, $packageModuleSpecifier, $isEntrypoint);
     }
 
-    public function getRootDirectory(): string
+    /**
+     * Converts the "path" string from an importmap entry to the filesystem path.
+     *
+     * The path may already be a filesystem path. But if it starts with ".",
+     * then the path is relative and the root directory is prepended.
+     */
+    public function convertPathToFilesystemPath(string $path): string
+    {
+        if (!str_starts_with($path, '.')) {
+            return $path;
+        }
+
+        return Path::join($this->getRootDirectory(), $path);
+    }
+
+    /**
+     * Converts a filesystem path to a relative path that can be used in the importmap.
+     *
+     * If no relative path could be created - e.g. because the path is not in
+     * the same directory/subdirectory as the root importmap.php file - null is returned.
+     */
+    public function convertFilesystemPathToPath(string $filesystemPath): ?string
+    {
+        $rootImportMapDir = realpath($this->getRootDirectory());
+        $filesystemPath = realpath($filesystemPath);
+        if (!str_starts_with($filesystemPath, $rootImportMapDir)) {
+            return null;
+        }
+
+        // remove the root directory, prepend "./" & normalize slashes
+        return './'.str_replace('\\', '/', substr($filesystemPath, \strlen($rootImportMapDir) + 1));
+    }
+
+    private function getRootDirectory(): string
     {
         return \dirname($this->importMapConfigPath);
     }

diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php
@@ -209,11 +209,7 @@ private function findAsset(string $path): ?MappedAsset
             return $asset;
         }
 
-        if (str_starts_with($path, '.')) {
-            $path = $this->importMapConfigReader->getRootDirectory().'/'.$path;
-        }
-
-        return $this->assetMapper->getAssetFromSourcePath($path);
+        return $this->assetMapper->getAssetFromSourcePath($this->importMapConfigReader->convertPathToFilesystemPath($path));
     }
 
     private function findEagerImports(MappedAsset $asset): array

diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php
@@ -152,11 +152,10 @@ private function requirePackages(array $packagesToRequire, ImportMapEntries $imp
                 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->importName));
             }
 
-            $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);
+            if (null !== $relativePath = $this->importMapConfigReader->convertFilesystemPathToPath($asset->sourcePath)) {
+                $path = $relativePath;
             }
 
             $newEntry = ImportMapEntry::createLocal(
@@ -213,10 +212,6 @@ private function findAsset(string $path): ?MappedAsset
             return $asset;
         }
 
-        if (str_starts_with($path, '.')) {
-            $path = $this->importMapConfigReader->getRootDirectory().'/'.$path;
-        }
-
-        return $this->assetMapper->getAssetFromSourcePath($path);
+        return $this->assetMapper->getAssetFromSourcePath($this->importMapConfigReader->convertPathToFilesystemPath($path));
     }
 }
diff --git a/src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php b/src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php
@@ -38,11 +38,20 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
             ->willReturnCallback(function ($importName) {
                 return match ($importName) {
                     'module_in_importmap_local_asset' => ImportMapEntry::createLocal('module_in_importmap_local_asset', ImportMapType::JS, 'module_in_importmap_local_asset.js', false),
-                    'module_in_importmap_remote' => ImportMapEntry::createRemote('module_in_importmap_remote', ImportMapType::JS, '/path/to/vendor/module_in_importmap_remote.js', '1.2.3', 'could_be_anything', false),
-                    '@popperjs/core' => ImportMapEntry::createRemote('@popperjs/core', ImportMapType::JS, '/path/to/vendor/@popperjs/core.js', '1.2.3', 'could_be_anything', false),
+                    'module_in_importmap_remote' => ImportMapEntry::createRemote('module_in_importmap_remote', ImportMapType::JS, './vendor/module_in_importmap_remote.js', '1.2.3', 'could_be_anything', false),
+                    '@popperjs/core' => ImportMapEntry::createRemote('@popperjs/core', ImportMapType::JS, '/project/assets/vendor/@popperjs/core.js', '1.2.3', 'could_be_anything', false),
                     default => null,
                 };
             });
+        $importMapConfigReader->expects($this->any())
+            ->method('convertPathToFilesystemPath')
+            ->willReturnCallback(function ($path) {
+                return match ($path) {
+                    './vendor/module_in_importmap_remote.js' => '/project/assets/vendor/module_in_importmap_remote.js',
+                    '/project/assets/vendor/@popperjs/core.js' => '/project/assets/vendor/@popperjs/core.js',
+                    default => throw new \RuntimeException(sprintf('Unexpected path "%s"', $path)),
+                };
+            });
 
         $assetMapper = $this->createMock(AssetMapperInterface::class);
         $assetMapper->expects($this->any())
@@ -61,8 +70,8 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
                     '/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'),
-                    '/path/to/vendor/module_in_importmap_remote.js' => new MappedAsset('module_in_importmap_remote.js', publicPathWithoutDigest: '/assets/module_in_importmap_remote.js'),
-                    '/path/to/vendor/@popperjs/core.js' => new MappedAsset('assets/vendor/@popperjs/core.js', publicPathWithoutDigest: '/assets/@popperjs/core.js'),
+                    '/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'),
                     default => null,
                 };
             });
@@ -439,7 +448,12 @@ public function testCompileHandlesCircularBareImportAssets()
        $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));
+            ->willReturn(ImportMapEntry::createRemote('@popperjs/core', ImportMapType::JS, './vendor/@popperjs/core.js', '1.2.3', 'could_be_anything', false));
+        $importMapConfigReader->expects($this->any())
+            ->method('convertPathToFilesystemPath')
+            ->with('./vendor/@popperjs/core.js')
+            ->willReturn('/path/to/vendor/@popperjs/core.js');
+
         $assetMapper = $this->createMock(AssetMapperInterface::class);
         $assetMapper->expects($this->once())
             ->method('getAssetFromSourcePath')

diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapConfigReaderTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapConfigReaderTest.php
@@ -109,10 +109,50 @@ public function testGetEntriesAndWriteEntries()
         $this->assertSame($originalImportMapData, $newImportMapData);
     }
 
-    public function testGetRootDirectory()
+    /**
+     * @dataProvider getPathToFilesystemPathTests
+     */
+    public function testConvertPathToFilesystemPath(string $path, string $expectedPath)
+    {
+        $configReader = new ImportMapConfigReader(realpath(__DIR__.'/../Fixtures/importmap.php'), $this->createMock(RemotePackageStorage::class));
+        // normalize path separators for comparison
+        $expectedPath = str_replace('\\', '/', $expectedPath);
+        $this->assertSame($expectedPath, $configReader->convertPathToFilesystemPath($path));
+    }
+
+    public static function getPathToFilesystemPathTests()
+    {
+        yield 'no change' => [
+            'path' => 'dir1/file2.js',
+            'expectedPath' => 'dir1/file2.js',
+        ];
+
+        yield 'prefixed with relative period' => [
+            'path' => './dir1/file2.js',
+            'expectedPath' => realpath(__DIR__.'/../Fixtures').'/dir1/file2.js',
+        ];
+    }
+
+    /**
+     * @dataProvider getFilesystemPathToPathTests
+     */
+    public function testConvertFilesystemPathToPath(string $path, ?string $expectedPath)
     {
         $configReader = new ImportMapConfigReader(__DIR__.'/../Fixtures/importmap.php', $this->createMock(RemotePackageStorage::class));
-        $this->assertSame(__DIR__.'/../Fixtures', $configReader->getRootDirectory());
+        $this->assertSame($expectedPath, $configReader->convertFilesystemPathToPath($path));
+    }
+
+    public static function getFilesystemPathToPathTests()
+    {
+        yield 'not in root directory' => [
+            'path' => __FILE__,
+            'expectedPath' => null,
+        ];
+
+        yield 'converted to relative path' => [
+            'path' => __DIR__.'/../Fixtures/dir1/file2.js',
+            'expectedPath' => './dir1/file2.js',
+        ];
     }
 
     public function testFindRootImportMapEntry()

diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php
@@ -23,6 +23,7 @@
 use Symfony\Component\AssetMapper\ImportMap\JavaScriptImport;
 use Symfony\Component\AssetMapper\MappedAsset;
 use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\Filesystem\Path;
 
 class ImportMapGeneratorTest extends TestCase
 {
@@ -252,8 +253,14 @@ public function testGetRawImportMapData(array $importMapEntries, array $mappedAs
         $this->mockImportMap($importMapEntries);
         $this->mockAssetMapper($mappedAssets);
         $this->configReader->expects($this->any())
-            ->method('getRootDirectory')
-            ->willReturn('/fake/root');
+            ->method('convertPathToFilesystemPath')
+            ->willReturnCallback(function (string $path) {
+                if (!str_starts_with($path, '.')) {
+                    return $path;
+                }
+
+                return Path::join('/fake/root', $path);
+            });
 
         $this->assertEquals($expectedData, $manager->getRawImportMapData());
     }

diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapManagerTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapManagerTest.php
@@ -75,8 +75,22 @@ public function testRequire(array $packages, int $expectedProviderPackageArgumen
         ;
 
         $this->configReader->expects($this->any())
-            ->method('getRootDirectory')
-            ->willReturn(self::$writableRoot);
+            ->method('convertPathToFilesystemPath')
+            ->willReturnCallback(function ($path) {
+                if (str_ends_with($path, 'some_file.js')) {
+                    return '/path/to/assets/some_file.js';
+                }
+
+                throw new \Exception(sprintf('Unexpected path "%s"', $path));
+            });
+        $this->configReader->expects($this->any())
+            ->method('convertFilesystemPathToPath')
+            ->willReturnCallback(function ($path) {
+                return match ($path) {
+                    '/path/to/assets/some_file.js' => './assets/some_file.js',
+                    default => throw new \Exception(sprintf('Unexpected path "%s"', $path)),
+                };
+            });
         $this->configReader->expects($this->once())
             ->method('getEntries')
             ->willReturn(new ImportMapEntries())
@@ -187,15 +201,15 @@ public static function getRequirePackageTests(): iterable
         ];
 
         yield 'single_package_with_a_path' => [
-            'packages' => [new PackageRequireOptions('some/module', path: self::$writableRoot.'/assets/some_file.js')],
-            'expectedProviderPackageArgumentCount' => 0,
-                'resolvedPackages' => [],
-            'expectedImportMap' => [
-                'some/module' => [
-                    // converted to relative path
-                    'path' => './assets/some_file.js',
-                ],
+        'packages' => [new PackageRequireOptions('some/module', path: self::$writableRoot.'/assets/some_file.js')],
+        'expectedProviderPackageArgumentCount' => 0,
+        'resolvedPackages' => [],
+        'expectedImportMap' => [
+            'some/module' => [
+                // converted to relative path
+                'path' => './assets/some_file.js',
             ],
+        ],
         ];
     }
 
@@ -289,10 +303,6 @@ public function testUpdateWithSpecificPackages()
 
         $this->remotePackageDownloader->expects($this->once())
             ->method('downloadPackages');
-
-        $this->configReader->expects($this->any())
-            ->method('getRootDirectory')
-            ->willReturn(self::$writableRoot);
         $this->configReader->expects($this->once())
             ->method('writeEntries')
             ->with($this->callback(function (ImportMapEntries $entries) {