8000 [AssetMapper] Also preloading non-local, imported modules · symfony/symfony@de7fba7 · GitHub
[go: up one dir, main page]
Skip to content

Commit de7fba7

Browse files
weaverryanweaverryan
committed
[AssetMapper] Also preloading non-local, imported modules
1 parent fa48cfa commit de7fba7

File tree

3 files changed

+66
-51
lines changed

3 files changed

+66
-51
lines changed

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

Lines changed: 62 additions & 47 deletions
< 8000 td data-grid-cell-id="diff-94f83e34e9499c6b59e119e5803cccb6e8c427269eb247a68d18547a7c39b649-74-56-2" data-line-anchor="diff-94f83e34e9499c6b59e119e5803cccb6e8c427269eb247a68d18547a7c39b649L74" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-deletionLine-bgColor, var(--diffBlob-deletion-bgColor-line));padding-right:24px" tabindex="-1" valign="top" class="focusable-grid-cell diff-text-cell left-side-diff-cell border-right left-side">-
$resolvedPath = $importMapEntry->path;
Original file line numberDiff line numberDiff line change
@@ -54,63 +54,30 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
5454
return $fullImportString;
5555
}
5656

57-
$isBareImport = !str_starts_with($importedModule, '.');
58-
if ($isBareImport) {
59-
// try to find a matching importmap.php entry that is "local"
60-
// if found, locate the MappedAsset and add it as a dependency
61-
// but avoid changing the import path
62-
$importMapEntry = $this->importMapManager->findRootImportMapEntry($importedModule);
63-
if (!$importMapEntry) {
64-
// don't warn on missing non-relative (bare) imports: these could be valid URLs
65-
66-
return $fullImportString;
67-
}
68-
69-
// avoid 3rd party modules (including downloaded modules)
70-
if ($importMapEntry->isRemote()) {
71-
return $fullImportString;
72-
}
73-
74
57+
$isRelativeImport = str_starts_with($importedModule, '.');
58+
if (!$isRelativeImport) {
59+
$dependentAsset = $this->findAssetForBareImport($importedModule, $assetMapper);
7560
} else {
76-
try {
77-
$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $importedModule);
78-
} catch (RuntimeException $e) {
79-
$this->handleMissingImport(sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);
80-
81-
return $fullImportString;
82-
}
83-
}
84-
85-
$dependentAsset = $assetMapper->getAsset($resolvedPath);
86-
87-
if (!$dependentAsset) {
88-
$message = sprintf('Unable to find asset "%s" imported from "%s".', $importedModule, $asset->sourcePath);
89-
90-
try {
91-
if (null !== $assetMapper->getAsset(sprintf('%s.js', $resolvedPath))) {
92-
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $importedModule);
93-
}
94-
} catch (CircularAssetsException $e) {
95-
// avoid circular error if there is self-referencing import comments
96-
}
97-
98-
$this->handleMissingImport($message);
99-
100-
return $fullImportString;
61+
$dependentAsset = $this->findAssetForRelativeImport($importedModule, $asset, $assetMapper);
10162
}
10263

103-
// We found the path! List it as a dependency.
104-
// This will cause the asset to be included in the importmap.
64+
// List as a JavaScript import.
65+
// This will cause the asset to be included in the importmap (for relative imports)
66+
// and will be used to generate the preloads in the importmap.
10567
$isLazy = str_contains($fullImportString, 'import(');
10668
$asset->addJavaScriptImport(new JavaScriptImport(
10769
$importedModule,
10870
$isLazy,
10971
$dependentAsset
11072
));
11173

112-
if ($isBareImport) {
113-
// do not adjust the path for bare imports
74+
if (!$isRelativeImport) {
75+
// do not adjust the path for non-relative (bare) imports
76+
return $fullImportString;
77+
}
78+
79+
if (!$dependentAsset) {
80+
// we can't find the asset: don't modify the import
11481
return $fullImportString;
11582
}
11683

@@ -171,4 +138,52 @@ private function isCommentedOut(mixed $offsetStart, string $fullContent): bool
171138

172139
return false;
173140
}
141+
142+
private function findAssetForBareImport(string $importedModule, AssetMapperInterface $assetMapper): ?MappedAsset
143+
{
144+
$importMapEntry = $this->importMapManager->findRootImportMapEntry($importedModule);
145+
if (!$importMapEntry) {
67E6 146+
// don't warn on missing non-relative (bare) imports: these could be valid URLs
147+
148+
return null;
149+
}
150+
151+
// remote entries have no MappedAsset
152+
if ($importMapEntry->isRemote()) {
153+
return null;
154+
}
155+
156+
return $assetMapper->getAsset($importMapEntry->path);
157+
}
158+
159+
private function findAssetForRelativeImport(string $importedModule, MappedAsset $asset, AssetMapperInterface $assetMapper): ?MappedAsset
160+
{
161+
try {
162+
$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $importedModule);
163+
} catch (RuntimeException $e) {
164+
$this->handleMissingImport(sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);
165+
166+
return null;
167+
}
168+
169+
$dependentAsset = $assetMapper->getAsset($resolvedPath);
170+
171+
if ($dependentAsset) {
172+
return $dependentAsset;
173+
}
174+
175+
$message = sprintf('Unable to find asset "%s" imported from "%s".', $importedModule, $asset->sourcePath);
176+
177+
try {
178+
if (null !== $assetMapper->getAsset(sprintf('%s.js', $resolvedPath))) {
179+
$message .= sprintf(' Try adding ".js" to the end of the import - i.e. "%s.js".', $importedModule);
180+
}
181+
} catch (CircularAssetsException $e) {
182+
// avoid circular error if there is self-referencing import comments
183+
}
184+
185+
$this->handleMissingImport($message);
186+
187+
return null;
188+
}
174189
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public function getImportMapData(array $entrypointNames): array
8484
}
8585

8686
if (!isset($rawImportMapData[$dependency])) {
87-
throw new \InvalidArgumentException(sprintf('The entrypoint "%s" depends on "%s", but that package is not in the importmap.', $entry, $dependency));
87+
// missing dependency - rely on browser or compilers to warn
88+
continue;
8889
}
8990

9091
if (false === ($rawImportMapData[$dependency]['preload'] ?? null)) {
@@ -379,7 +380,6 @@ private function requirePackages(array $packagesToRequire, ImportMapEntries $imp
379380
url: $resolvedPackage->url,
380381
isDownloaded: $resolvedPackage->requireOptions->download,
381382
type: $type,
382-
isEntrypoint: true,
383383
);
384384
$importMapEntries->add($newEntry);
385385
$addedEntries[] = $newEntry;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,10 @@ public static function provideCompileTests(): iterable
177177
'expectedJavaScriptImports' => ['./styles.css' => ['lazy' => false, 'asset' => 'styles.css']],
178178
];
179179

180-
yield 'importing_non_existent_file_without_strict_mode_is_ignored' => [
180+
yield 'importing_non_existent_file_without_strict_mode_is_ignored_still_listed_as_an_import' => [
181181
'sourceLogicalName' => 'app.js',
182182
'input' => "import './non-existent.js';",
183-
'expectedJavaScriptImports' => [],
183+
'expectedJavaScriptImports' => ['./non-existent.js' => ['lazy' => false, 'asset' => null]],
184184
];
185185

186186
yield 'single_line_comment_at_start_ignored' => [

0 commit comments

Comments
 (0)
0