Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f1708aa

Browse filesBrowse files
weaverryanfabpot
weaverryan
authored and
fabpot
committed
[AssetMapper] Fix file deleting errors & remove nullable MappedAsset on JS import
1 parent eaff34a commit f1708aa
Copy full SHA for f1708aa

File tree

Expand file treeCollapse file tree

9 files changed

+56
-42
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+56
-42
lines changed

‎src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Compiler/JavaScriptImportPathCompiler.php
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,19 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
6161
$dependentAsset = $this->findAssetForRelativeImport($importedModule, $asset, $assetMapper);
6262
}
6363

64+
if (!$dependentAsset) {
65+
return $fullImportString;
66+
}
67+
6468
// List as a JavaScript import.
6569
// This will cause the asset to be included in the importmap (for relative imports)
6670
// and will be used to generate the preloads in the importmap.
6771
$isLazy = str_contains($fullImportString, 'import(');
68-
$addToImportMap = $isRelativeImport && $dependentAsset;
72+
$addToImportMap = $isRelativeImport;
6973
$asset->addJavaScriptImport(new JavaScriptImport(
7074
$addToImportMap ? $dependentAsset->publicPathWithoutDigest : $importedModule,
71-
$isLazy,
7275
$dependentAsset,
76+
$isLazy,
7377
$addToImportMap,
7478
));
7579

‎src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Factory/CachedMappedAssetFactory.php
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Symfony\Component\AssetMapper\MappedAsset;
1515
use Symfony\Component\Config\ConfigCache;
1616
use Symfony\Component\Config\Resource\DirectoryResource;
17+
use Symfony\Component\Config\Resource\FileExistenceResource;
1718
use Symfony\Component\Config\Resource\FileResource;
1819
use Symfony\Component\Config\Resource\ResourceInterface;
1920

@@ -67,6 +68,10 @@ private function collectResourcesFromAsset(MappedAsset $mappedAsset): array
6768
$resources = array_merge($resources, $this->collectResourcesFromAsset($assetDependency));
6869
}
6970

71+
foreach ($mappedAsset->getJavaScriptImports() as $import) {
72+
$resources[] = new FileExistenceResource($import->asset->sourcePath);
73+
}
74+
7075
return $resources;
7176
}
7277
}

‎src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php
+3-5Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ private function addImplicitEntries(ImportMapEntry $entry, array $currentImportE
178178
}
179179

180180
// check if this import requires an automatic importmap entry
181-
if ($javaScriptImport->addImplicitlyToImportMap && $javaScriptImport->asset) {
181+
if ($javaScriptImport->addImplicitlyToImportMap) {
182182
$nextEntry = ImportMapEntry::createLocal(
183183
$importName,
184184
ImportMapType::tryFrom($javaScriptImport->asset->publicExtension) ?: ImportMapType::JS,
@@ -226,10 +226,8 @@ private function findEagerImports(MappedAsset $asset): array
226226

227227
$dependencies[] = $javaScriptImport->importName;
228228

229-
// the import is for a MappedAsset? Follow its imports!
230-
if ($javaScriptImport->asset) {
231-
$dependencies = array_merge($dependencies, $this->findEagerImports($javaScriptImport->asset));
232-
}
229+
// Follow its imports!
230+
$dependencies = array_merge($dependencies, $this->findEagerImports($javaScriptImport->asset));
233231
}
234232

235233
return $dependencies;

‎src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/ImportMap/JavaScriptImport.php
+4-8Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,15 @@
1919
final class JavaScriptImport
2020
{
2121
/**
22-
* @param string $importName The name of the import needed in the importmap, e.g. "/foo.js" or "react"
23-
* @param bool $isLazy Whether this import was lazy or eager
24-
* @param MappedAsset|null $asset The asset that was imported, if known - needed to add to the importmap, also used to find further imports for preloading
25-
* @param bool $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
22+
* @param string $importName The name of the import needed in the importmap, e.g. "/foo.js" or "react"
23+
* @param MappedAsset $asset The asset that was imported
24+
* @param bool $addImplicitlyToImportMap Whether this import should be added to the importmap automatically
2625
*/
2726
public function __construct(
2827
public readonly string $importName,
28+
public readonly MappedAsset $asset,
2929
public readonly bool $isLazy = false,
30-
public readonly ?MappedAsset $asset = null,
3130
public bool $addImplicitlyToImportMap = false,
3231
) {
33-
if (null === $asset && $addImplicitlyToImportMap) {
34-
throw new \LogicException(sprintf('The "%s" import cannot be automatically added to the importmap without an asset.', $this->importName));
35-
}
3632
}
3733
}

‎src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Tests/Compiler/JavaScriptImportPathCompilerTest.php
+11-5Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function testCompileFindsCorrectImports(string $input, array $expectedJav
7272
$this->assertSame($input, $compiler->compile($input, $asset, $assetMapper));
7373
$actualImports = [];
7474
foreach ($asset->getJavaScriptImports() as $import) {
75-
$actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->asset?->logicalPath, 'add' => $import->addImplicitlyToImportMap];
75+
$actualImports[$import->importName] = ['lazy' => $import->isLazy, 'asset' => $import->asset->logicalPath, 'add' => $import->addImplicitlyToImportMap];
7676
}
7777

7878
$this->assertEquals($expectedJavaScriptImports, $actualImports);
@@ -172,9 +172,10 @@ public static function provideCompileTests(): iterable
172172
'expectedJavaScriptImports' => ['/assets/styles.css' => ['lazy' => false, 'asset' => 'styles.css', 'add' => true]],
173173
];
174174

175-
yield 'importing_non_existent_file_without_strict_mode_is_ignored_still_listed_as_an_import' => [
175+
yield 'importing_non_existent_file_without_strict_mode_is_ignored_and_no_import_added' => [
176+
'sourceLogicalName' => 'app.js',
176177
'input' => "import './non-existent.js';",
177-
'expectedJavaScriptImports' => ['./non-existent.js' => ['lazy' => false, 'asset' => null, 'add' => false]],
178+
'expectedJavaScriptImports' => [],
178179
];
179180

180181
yield 'single_line_comment_at_start_ignored' => [
@@ -262,7 +263,7 @@ public static function provideCompileTests(): iterable
262263

263264
yield 'bare_import_not_in_importmap' => [
264265
'input' => 'import "some_module";',
265-
'expectedJavaScriptImports' => ['some_module' => ['lazy' => false, 'asset' => null, 'add' => false]],
266+
'expectedJavaScriptImports' => [],
266267
];
267268

268269
yield 'bare_import_in_importmap_with_local_asset' => [
@@ -275,9 +276,14 @@ public static function provideCompileTests(): iterable
275276
'expectedJavaScriptImports' => ['module_in_importmap_remote' => ['lazy' => false, 'asset' => 'module_in_importmap_remote.js', 'add' => false]],
276277
];
277278

279+
<<<<<<< HEAD
278280
yield 'absolute_import_added_as_dependency_only' => [
281+
=======
282+
yield 'absolute_import_ignored_and_no_dependency_added' => [
283+
'sourceLogicalName' => 'app.js',
284+
>>>>>>> a79f543f8f ([AssetMapper] Fix file deleting errors & remove nullable MappedAsset on JS import)
279285
'input' => 'import "https://example.com/module.js";',
280-
'expectedJavaScriptImports' => ['https://example.com/module.js' => ['lazy' => false, 'asset' => null, 'add' => false]],
286+
'expectedJavaScriptImports' => [],
281287
];
282288

283289
yield 'bare_import_with_minimal_spaces' => [

‎src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Tests/Factory/CachedMappedAssetFactoryTest.php
+7-2Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\AssetMapper\Factory\CachedMappedAssetFactory;
1616
use Symfony\Component\AssetMapper\Factory\MappedAssetFactoryInterface;
17+
use Symfony\Component\AssetMapper\ImportMap\JavaScriptImport;
1718
use Symfony\Component\AssetMapper\MappedAsset;
1819
use Symfony\Component\Config\ConfigCache;
1920
use Symfony\Component\Config\Resource\DirectoryResource;
21+
use Symfony\Component\Config\Resource\FileExistenceResource;
2022
use Symfony\Component\Config\Resource\FileResource;
2123
use Symfony\Component\Filesystem\Filesystem;
2224

@@ -89,9 +91,11 @@ public function testAssetConfigCacheResourceContainsDependencies()
8991
$mappedAsset = new MappedAsset('file1.css', $sourcePath, content: 'cached content');
9092

9193
$dependentOnContentAsset = new MappedAsset('file3.css', realpath(__DIR__.'/../Fixtures/dir2/file3.css'));
92-
9394
$deeplyNestedAsset = new MappedAsset('file4.js', realpath(__DIR__.'/../Fixtures/dir2/file4.js'));
9495

96+
$file6Asset = new MappedAsset('file6.js', realpath(__DIR__.'/../Fixtures/dir2/subdir/file6.js'));
97+
$deeplyNestedAsset->addJavaScriptImport(new JavaScriptImport('file6', asset: $file6Asset));
98+
9599
$dependentOnContentAsset->addDependency($deeplyNestedAsset);
96100
$mappedAsset->addDependency($dependentOnContentAsset);
97101

@@ -112,14 +116,15 @@ public function testAssetConfigCacheResourceContainsDependencies()
112116
$cachedFactory->createMappedAsset('file1.css', $sourcePath);
113117

114118
$configCacheMetadata = $this->loadConfigCacheMetadataFor($mappedAsset);
115-
$this->assertCount(5, $configCacheMetadata);
119+
$this->assertCount(6, $configCacheMetadata);
116120
$this->assertInstanceOf(FileResource::class, $configCacheMetadata[0]);
117121
$this->assertInstanceOf(DirectoryResource::class, $configCacheMetadata[1]);
118122
$this->assertInstanceOf(FileResource::class, $configCacheMetadata[2]);
119123
$this->assertSame(realpath(__DIR__.'/../Fixtures/importmap.php'), $configCacheMetadata[0]->getResource());
120124
$this->assertSame($mappedAsset->sourcePath, $configCacheMetadata[2]->getResource());
121125
$this->assertSame($dependentOnContentAsset->sourcePath, $configCacheMetadata[3]->getResource());
122126
$this->assertSame($deeplyNestedAsset->sourcePath, $configCacheMetadata[4]->getResource());
127+
$this->assertInstanceOf(FileExistenceResource::class, $configCacheMetadata[5]);
123128
}
124129

125130
private function loadConfigCacheMetadataFor(MappedAsset $mappedAsset): array

‎src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php
+18-18Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -139,25 +139,25 @@ public function testGetImportMapData()
139139
'entry1.js',
140140
publicPath: '/assets/entry1-d1g35t.js',
141141
javaScriptImports: [
142-
new JavaScriptImport('/assets/imported_file1.js', isLazy: false, asset: $importedFile1, addImplicitlyToImportMap: true),
143-
new JavaScriptImport('/assets/styles/file1.css', isLazy: false, asset: $importedCss1, addImplicitlyToImportMap: true),
144-
new JavaScriptImport('normal_js_file', isLazy: false, asset: $normalJsFile),
142+
new JavaScriptImport('/assets/imported_file1.js', asset: $importedFile1, isLazy: false, addImplicitlyToImportMap: true),
143+
new JavaScriptImport('/assets/styles/file1.css', asset: $importedCss1, isLazy: false, addImplicitlyToImportMap: true),
144+
new JavaScriptImport('normal_js_file', asset: $normalJsFile, isLazy: false),
145145
]
146146
),
147147
new MappedAsset(
148148
'entry2.js',
149149
publicPath: '/assets/entry2-d1g35t.js',
150150
javaScriptImports: [
151-
new JavaScriptImport('/assets/imported_file2.js', isLazy: false, asset: $importedFile2, addImplicitlyToImportMap: true),
152-
new JavaScriptImport('css_in_importmap', isLazy: false, asset: $importedCssInImportmap),
153-
new JavaScriptImport('/assets/styles/file2.css', isLazy: false, asset: $importedCss2, addImplicitlyToImportMap: true),
151+
new JavaScriptImport('/assets/imported_file2.js', asset: $importedFile2, isLazy: false, addImplicitlyToImportMap: true),
152+
new JavaScriptImport('css_in_importmap', asset: $importedCssInImportmap, isLazy: false),
153+
new JavaScriptImport('/assets/styles/file2.css', asset: $importedCss2, isLazy: false, addImplicitlyToImportMap: true),
154154
]
155155
),
156156
new MappedAsset(
157157
'entry3.js',
158158
publicPath: '/assets/entry3-d1g35t.js',
159159
javaScriptImports: [
160-
new JavaScriptImport('/assets/imported_file3.js', isLazy: false, asset: $importedFile3),
160+
new JavaScriptImport('/assets/imported_file3.js', asset: $importedFile3, isLazy: false),
161161
],
162162
),
163163
$importedFile1,
@@ -342,7 +342,7 @@ public function getRawImportMapDataTests(): iterable
342342
new MappedAsset(
343343
'app.js',
344344
publicPath: '/assets/app-d1g3st.js',
345-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: false, asset: $simpleAsset, addImplicitlyToImportMap: true)]
345+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)]
346346
),
347347
$simpleAsset,
348348
],
@@ -371,7 +371,7 @@ public function getRawImportMapDataTests(): iterable
371371
'app.js',
372372
sourcePath: '/assets/vendor/bootstrap.js',
373373
publicPath: '/assets/vendor/bootstrap-d1g3st.js',
374-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: false, asset: $simpleAsset, addImplicitlyToImportMap: true)]
374+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)]
375375
),
376376
$simpleAsset,
377377
],
@@ -391,7 +391,7 @@ public function getRawImportMapDataTests(): iterable
391391
'imports_simple.js',
392392
publicPathWithoutDigest: '/assets/imports_simple.js',
393393
publicPath: '/assets/imports_simple-d1g3st.js',
394-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: false, asset: $simpleAsset, addImplicitlyToImportMap: true)]
394+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false, addImplicitlyToImportMap: true)]
395395
);
396396
yield 'it processes imports recursively' => [
397397
[
@@ -404,7 +404,7 @@ public function getRawImportMapDataTests(): iterable
404404
new MappedAsset(
405405
'app.js',
406406
publicPath: '/assets/app-d1g3st.js',
407-
javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', isLazy: true, asset: $eagerImportsSimpleAsset, addImplicitlyToImportMap: true)]
407+
javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', asset: $eagerImportsSimpleAsset, isLazy: true, addImplicitlyToImportMap: true)]
408408
),
409409
$eagerImportsSimpleAsset,
410410
$simpleAsset,
@@ -440,7 +440,7 @@ public function getRawImportMapDataTests(): iterable
440440
new MappedAsset(
441441
'app.js',
442442
publicPath: '/assets/app-d1g3st.js',
443-
javaScriptImports: [new JavaScriptImport('imports_simple', isLazy: true, asset: $eagerImportsSimpleAsset, addImplicitlyToImportMap: false)]
443+
javaScriptImports: [new JavaScriptImport('imports_simple', asset: $eagerImportsSimpleAsset, isLazy: true, addImplicitlyToImportMap: false)]
444444
),
445445
$eagerImportsSimpleAsset,
446446
$simpleAsset,
@@ -472,7 +472,7 @@ public function getRawImportMapDataTests(): iterable
472472
new MappedAsset(
473473
'app.js',
474474
publicPath: '/assets/app-d1g3st.js',
475-
javaScriptImports: [new JavaScriptImport('simple', isLazy: false, asset: $simpleAsset)]
475+
javaScriptImports: [new JavaScriptImport('simple', asset: $simpleAsset, isLazy: false)]
476476
),
477477
$simpleAsset,
478478
],
@@ -609,7 +609,7 @@ public function getEagerEntrypointImportsTests(): iterable
609609
new MappedAsset(
610610
'app.js',
611611
publicPath: '/assets/app.js',
612-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: false, asset: $simpleAsset)]
612+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false)]
613613
),
614614
['/assets/simple.js'], // path is the key in the importmap
615615
];
@@ -618,7 +618,7 @@ public function getEagerEntrypointImportsTests(): iterable
618618
new MappedAsset(
619619
'app.js',
620620
publicPath: '/assets/app.js',
621-
javaScriptImports: [new JavaScriptImport('simple', isLazy: false, asset: $simpleAsset)]
621+
javaScriptImports: [new JavaScriptImport('simple', asset: $simpleAsset, isLazy: false)]
622622
),
623623
['simple'], // path is the key in the importmap
624624
];
@@ -627,21 +627,21 @@ public function getEagerEntrypointImportsTests(): iterable
627627
new MappedAsset(
628628
'app.js',
629629
publicPath: '/assets/app.js',
630-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: true, asset: $simpleAsset)]
630+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: true)]
631631
),
632632
[],
633633
];
634634

635635
$importsSimpleAsset = new MappedAsset(
636636
'imports_simple.js',
637637
publicPathWithoutDigest: '/assets/imports_simple.js',
638-
javaScriptImports: [new JavaScriptImport('/assets/simple.js', isLazy: false, asset: $simpleAsset)]
638+
javaScriptImports: [new JavaScriptImport('/assets/simple.js', asset: $simpleAsset, isLazy: false)]
639639
);
640640
yield 'an entry follows through dependencies recursively' => [
641641
new MappedAsset(
642642
'app.js',
643643
publicPath: '/assets/app.js',
644-
javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', isLazy: false, asset: $importsSimpleAsset)]
644+
javaScriptImports: [new JavaScriptImport('/assets/imports_simple.js', asset: $importsSimpleAsset, isLazy: false)]
645645
),
646646
['/assets/imports_simple.js', '/assets/simple.js'],
647647
];

‎src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Tests/ImportMap/JavaScriptImportTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class JavaScriptImportTest extends TestCase
2020
public function testBasicConstruction()
2121
{
2222
$asset = new MappedAsset('the-asset');
23-
$import = new JavaScriptImport('the-import', true, $asset, true);
23+
$import = new JavaScriptImport('the-import', $asset, true, true);
2424

2525
$this->assertSame('the-import', $import->importName);
2626
$this->assertTrue($import->isLazy);

‎src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/AssetMapper/Tests/MappedAssetTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function testAddJavaScriptImports()
5858
$mainAsset = new MappedAsset('file.js');
5959

6060
$assetFoo = new MappedAsset('foo.js');
61-
$javaScriptImport = new JavaScriptImport('/the_import', isLazy: true, asset: $assetFoo);
61+
$javaScriptImport = new JavaScriptImport('/the_import', asset: $assetFoo, isLazy: true);
6262
$mainAsset->addJavaScriptImport($javaScriptImport);
6363

6464
$this->assertSame([$javaScriptImport], $mainAsset->getJavaScriptImports());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.