From bb5ef88ac666227b3b91d8585d5f22499ec93e23 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 9 Nov 2023 15:02:25 -0500 Subject: [PATCH] [AssetMapper] Avoid storing & caching MappedAsset objects The problem is that these could get out of date, allowing asset Foo to have a JavaScript import for the MappedAsset for Bar, but it is out of date because Bar has since changed. --- .../Compiler/JavaScriptImportPathCompiler.php | 3 +- .../Factory/CachedMappedAssetFactory.php | 2 +- .../ImportMap/ImportMapGenerator.php | 15 ++++- .../ImportMap/JavaScriptImport.php | 11 ++-- .../Component/AssetMapper/MappedAsset.php | 2 + .../JavaScriptImportPathCompilerTest.php | 40 ++++++------ .../Factory/CachedMappedAssetFactoryTest.php | 2 +- .../ImportMap/ImportMapGeneratorTest.php | 61 ++++++++++++------- .../Tests/ImportMap/JavaScriptImportTest.php | 7 +-- .../AssetMapper/Tests/MappedAssetTest.php | 3 +- 10 files changed, 87 insertions(+), 59 deletions(-) diff --git a/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php b/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php index aad37f40f7e5..93e04846fce9 100644 --- a/src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php +++ b/src/Symfony/Component/AssetMapper/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, )); diff --git a/src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php b/src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php index f561ffd43399..eff109c22624 100644 --- a/src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php +++ b/src/Symfony/Component/AssetMapper/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; diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php index 72df22e386bf..2ec5941323e1 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php +++ b/src/Symfony/Component/AssetMapper/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; diff --git a/src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php b/src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php index 6256c1eca180..3b1ccf705c10 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php +++ b/src/Symfony/Component/AssetMapper/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, ) { diff --git a/src/Symfony/Component/AssetMapper/MappedAsset.php b/src/Symfony/Component/AssetMapper/MappedAsset.php index 66959de7636b..0962ec1c3fb7 100644 --- a/src/Symfony/Component/AssetMapper/MappedAsset.php +++ b/src/Symfony/Component/AssetMapper/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 diff --git a/src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php b/src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php index b9761f3a86df..99673d1a042a 100644 --- a/src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php +++ b/src/Symfony/Component/AssetMapper/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, }; } diff --git a/src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php b/src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php index 0712fe0a0801..62a37fe837b9 100644 --- a/src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php +++ b/src/Symfony/Component/AssetMapper/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); diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php index 4643a521db5e..31c0855d8f02 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php @@ -97,68 +97,79 @@ public function testGetImportMapData() $importedFile1 = new MappedAsset( 'imported_file1.js', + '/path/to/imported_file1.js', publicPathWithoutDigest: '/assets/imported_file1.js', publicPath: '/assets/imported_file1-d1g35t.js', ); $importedFile2 = new MappedAsset( 'imported_file2.js', + '/path/to/imported_file2.js', publicPathWithoutDigest: '/assets/imported_file2.js', publicPath: '/assets/imported_file2-d1g35t.js', ); $importedFile3 = new MappedAsset( 'imported_file3.js', + '/path/to/imported_file3.js', publicPathWithoutDigest: '/assets/imported_file3.js', publicPath: '/assets/imported_file3-d1g35t.js', ); $normalJsFile = new MappedAsset( 'normal_js_file.js', + '/path/to/normal_js_file.js', publicPathWithoutDigest: '/assets/normal_js_file.js', publicPath: '/assets/normal_js_file-d1g35t.js', ); $importedCss1 = new MappedAsset( 'styles/file1.css', + '/path/to/styles/file1.css', publicPathWithoutDigest: '/assets/styles/file1.css', publicPath: '/assets/styles/file1-d1g35t.css', ); $importedCss2 = new MappedAsset( 'styles/file2.css', + '/path/to/styles/file2.css', publicPathWithoutDigest: '/assets/styles/file2.css', publicPath: '/assets/styles/file2-d1g35t.css', ); $importedCssInImportmap = new MappedAsset( 'styles/css_in_importmap.css', + '/path/to/styles/css_in_importmap.css', publicPathWithoutDigest: '/assets/styles/css_in_importmap.css', publicPath: '/assets/styles/css_in_importmap-d1g35t.css', ); $neverImportedCss = new MappedAsset( 'styles/never_imported_css.css', + '/path/to/styles/never_imported_css.css', publicPathWithoutDigest: '/assets/styles/never_imported_css.css', publicPath: '/assets/styles/never_imported_css-d1g35t.css', ); $this->mockAssetMapper([ new MappedAsset( 'entry1.js', + '/path/to/entry1.js', publicPath: '/assets/entry1-d1g35t.js', javaScriptImports: [ - new JavaScriptImport('/assets/imported_file1.js', asset: $importedFile1, isLazy: false, addImplicitlyToImportMap: true), - new JavaScriptImport('/assets/styles/file1.css', asset: $importedCss1, isLazy: false, addImplicitlyToImportMap: true), - new JavaScriptImport('normal_js_file', asset: $normalJsFile, isLazy: false), + new JavaScriptImport('/assets/imported_file1.js', assetLogicalPath: $importedFile1->logicalPath, assetSourcePath: $importedFile1->sourcePath, isLazy: false, addImplicitlyToImportMap: true), + new JavaScriptImport('/assets/styles/file1.css', assetLogicalPath: $importedCss1->logicalPath, assetSourcePath: $importedCss1->sourcePath, isLazy: false, addImplicitlyToImportMap: true), + new JavaScriptImport('normal_js_file', assetLogicalPath: $normalJsFile->logicalPath, assetSourcePath: $normalJsFile->sourcePath, isLazy: false), ] ), new MappedAsset( 'entry2.js', + '/path/to/entry2.js', publicPath: '/assets/entry2-d1g35t.js', javaScriptImports: [ - new JavaScriptImport('/assets/imported_file2.js', asset: $importedFile2, isLazy: false, addImplicitlyToImportMap: true), - new JavaScriptImport('css_in_importmap', asset: $importedCssInImportmap, isLazy: false), - new JavaScriptImport('/assets/styles/file2.css', asset: $importedCss2, isLazy: false, addImplicitlyToImportMap: true), + new JavaScriptImport('/assets/imported_file2.js', assetLogicalPath: $importedFile2->logicalPath, assetSourcePath: $importedFile2->sourcePath, isLazy: false, addImplicitlyToImportMap: true), + new JavaScriptImport('css_in_importmap', assetLogicalPath: $importedCssInImportmap->logicalPath, assetSourcePath: $importedCssInImportmap->sourcePath, isLazy: false), + new JavaScriptImport('/assets/styles/file2.css', assetLogicalPath: $importedCss2->logicalPath, assetSourcePath: $importedCss2->sourcePath, isLazy: false, addImplicitlyToImportMap: true), ] ), new MappedAsset( 'entry3.js', + '/path/to/entry3.js', publicPath: '/assets/entry3-d1g35t.js', javaScriptImports: [ - new JavaScriptImport('/assets/imported_file3.js', asset: $importedFile3, isLazy: false), + new JavaScriptImport('/assets/imported_file3.js', assetLogicalPath: $importedFile3->logicalPath, assetSourcePath: $importedFile3->sourcePath, isLazy: false), ], ), $importedFile1, @@ -335,6 +346,7 @@ public function getRawImportMapDataTests(): iterable $simpleAsset = new MappedAsset( 'simple.js', + '/path/to/simple.js', publicPathWithoutDigest: '/assets/simple.js', publicPath: '/assets/simple-d1g3st.js', ); @@ -349,7 +361,7 @@ public function getRawImportMapDataTests(): iterable new MappedAsset( 'app.js', publicPath: '/assets/app-d1g3st.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)] ), $simpleAsset, ], @@ -378,7 +390,7 @@ public function getRawImportMapDataTests(): iterable 'app.js', sourcePath: '/assets/vendor/bootstrap.js', publicPath: '/assets/vendor/bootstrap-d1g3st.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)] ), $simpleAsset, ], @@ -396,9 +408,10 @@ public function getRawImportMapDataTests(): iterable $eagerImportsSimpleAsset = new MappedAsset( 'imports_simple.js', + '/path/to/imports_simple.js', publicPathWithoutDigest: '/assets/imports_simple.js', publicPath: '/assets/imports_simple-d1g3st.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)] ); yield 'it processes imports recursively' => [ [ @@ -411,7 +424,7 @@ public function getRawImportMapDataTests(): iterable new MappedAsset( 'app.js', publicPath: '/assets/app-d1g3st.js', - javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', asset: $eagerImportsSimpleAsset, isLazy: true, addImplicitlyToImportMap: true)] + javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', assetLogicalPath: $eagerImportsSimpleAsset->logicalPath, assetSourcePath: $eagerImportsSimpleAsset->sourcePath, isLazy: true, addImplicitlyToImportMap: true)] ), $eagerImportsSimpleAsset, $simpleAsset, @@ -447,7 +460,7 @@ public function getRawImportMapDataTests(): iterable new MappedAsset( 'app.js', publicPath: '/assets/app-d1g3st.js', - javaScriptImports: [new JavaScriptImport('imports_simple', asset: $eagerImportsSimpleAsset, isLazy: true, addImplicitlyToImportMap: false)] + javaScriptImports: [new JavaScriptImport('imports_simple', assetLogicalPath: $eagerImportsSimpleAsset->logicalPath, assetSourcePath: $eagerImportsSimpleAsset->logicalPath, isLazy: true, addImplicitlyToImportMap: false)] ), $eagerImportsSimpleAsset, $simpleAsset, @@ -479,7 +492,7 @@ public function getRawImportMapDataTests(): iterable new MappedAsset( 'app.js', publicPath: '/assets/app-d1g3st.js', - javaScriptImports: [new JavaScriptImport('simple', asset: $simpleAsset, isLazy: false)] + javaScriptImports: [new JavaScriptImport('simple', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false)] ), $simpleAsset, ], @@ -503,7 +516,7 @@ public function getRawImportMapDataTests(): iterable new MappedAsset( 'app.css', publicPath: '/assets/app-d1g3st.css', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath)] ), ], [ @@ -586,10 +599,10 @@ public function testGetRawImportDataUsesCacheFile() /** * @dataProvider getEagerEntrypointImportsTests */ - public function testFindEagerEntrypointImports(MappedAsset $entryAsset, array $expected) + public function testFindEagerEntrypointImports(MappedAsset $entryAsset, array $expected, array $mappedAssets = []) { $manager = $this->createImportMapGenerator(); - $this->mockAssetMapper([$entryAsset]); + $this->mockAssetMapper([$entryAsset, ...$mappedAssets]); // put the entry asset in the importmap $this->mockImportMap([ ImportMapEntry::createLocal('the_entrypoint_name', ImportMapType::JS, path: $entryAsset->logicalPath, isEntrypoint: true), @@ -610,47 +623,53 @@ public function getEagerEntrypointImportsTests(): iterable $simpleAsset = new MappedAsset( 'simple.js', + '/path/to/simple.js', publicPathWithoutDigest: '/assets/simple.js', ); yield 'an entry with a non-lazy dependency is included' => [ new MappedAsset( 'app.js', publicPath: '/assets/app.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false)] ), ['/assets/simple.js'], // path is the key in the importmap + [$simpleAsset], ]; yield 'an entry with a non-lazy dependency with module name is included' => [ new MappedAsset( 'app.js', publicPath: '/assets/app.js', - javaScriptImports: [new JavaScriptImport('simple', asset: $simpleAsset, isLazy: false)] + javaScriptImports: [new JavaScriptImport('simple', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false)] ), ['simple'], // path is the key in the importmap + [$simpleAsset], ]; yield 'an entry with a lazy dependency is not included' => [ new MappedAsset( 'app.js', publicPath: '/assets/app.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: true)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: true)] ), [], + [$simpleAsset], ]; $importsSimpleAsset = new MappedAsset( 'imports_simple.js', + '/path/to/imports_simple.js', publicPathWithoutDigest: '/assets/imports_simple.js', - javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false)] + javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false)] ); yield 'an entry follows through dependencies recursively' => [ new MappedAsset( 'app.js', publicPath: '/assets/app.js', - javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', asset: $importsSimpleAsset, isLazy: false)] + javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', assetLogicalPath: $importsSimpleAsset->logicalPath, assetSourcePath: $importsSimpleAsset->sourcePath, isLazy: false)] ), ['/assets/imports_simple.js', '/assets/simple.js'], + [$simpleAsset, $importsSimpleAsset], ]; } diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php index f4d4481e5d68..864765936eca 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php @@ -13,18 +13,17 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\AssetMapper\ImportMap\JavaScriptImport; -use Symfony\Component\AssetMapper\MappedAsset; class JavaScriptImportTest extends TestCase { public function testBasicConstruction() { - $asset = new MappedAsset('the-asset'); - $import = new JavaScriptImport('the-import', $asset, true, true); + $import = new JavaScriptImport('the-import', 'the-asset', '/path/to/the-asset', true, true); $this->assertSame('the-import', $import->importName); $this->assertTrue($import->isLazy); - $this->assertSame($asset, $import->asset); + $this->assertSame('the-asset', $import->assetLogicalPath); + $this->assertSame('/path/to/the-asset', $import->assetSourcePath); $this->assertTrue($import->addImplicitlyToImportMap); } } diff --git a/src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php b/src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php index 062e29f11e83..e2bf6c1f22c5 100644 --- a/src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php @@ -57,8 +57,7 @@ public function testAddJavaScriptImports() { $mainAsset = new MappedAsset('file.js'); - $assetFoo = new MappedAsset('foo.js'); - $javaScriptImport = new JavaScriptImport('/the_import', asset: $assetFoo, isLazy: true); + $javaScriptImport = new JavaScriptImport('/the_import', assetLogicalPath: 'foo.js', assetSourcePath: '/path/to/foo.js', isLazy: true); $mainAsset->addJavaScriptImport($javaScriptImport); $this->assertSame([$javaScriptImport], $mainAsset->getJavaScriptImports());