Skip to content

Commit

Permalink
bug #52702 [AssetMapper] Fix eager imports are not deduplicated (smna…
Browse files Browse the repository at this point in the history
…ndre)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Fix eager imports are not deduplicated

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes ?
| New feature?  | no
| Deprecations? | no
| Issues        | Fix
| License       | MIT

While recursively collecting eager imports, there may be duplicates.

```
app.js
L  A_1.js
    L B.js
L  A_2.js
    L B.js
```

Removing them early avoid a lot of useless computation afterwards (and eventual side effects)

Exemple with the generated app.entrypoint.json of the Symfony UX website
- before: 78 entries
- after: 53 entries

Commits
-------

e76b24b [AssetMapper] Fix eager imports are not deduplicated
  • Loading branch information
fabpot committed Nov 25, 2023
2 parents 0c9985c + e76b24b commit 77189f6
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 11 deletions.
33 changes: 22 additions & 11 deletions src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php
Expand Up @@ -217,25 +217,36 @@ private function findAsset(string $path): ?MappedAsset
return $this->assetMapper->getAssetFromSourcePath($this->importMapConfigReader->convertPathToFilesystemPath($path));
}

/**
* Finds recursively all the non-lazy modules imported by an asset.
*
* @return array<string> The array of deduplicated import names
*/
private function findEagerImports(MappedAsset $asset): array
{
$dependencies = [];
foreach ($asset->getJavaScriptImports() as $javaScriptImport) {
if ($javaScriptImport->isLazy) {
continue;
}
$queue = [$asset];

$dependencies[] = $javaScriptImport->importName;
while ($asset = array_shift($queue)) {
foreach ($asset->getJavaScriptImports() as $javaScriptImport) {
if ($javaScriptImport->isLazy) {
continue;
}
if (isset($dependencies[$javaScriptImport->importName])) {
continue;
}
$dependencies[$javaScriptImport->importName] = true;

// Follow its imports!
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));
// Follow its imports!
if (!$javaScriptAsset = $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 JavaScript asset "%s" (imported in "%s") in asset mapper.', $javaScriptImport->assetLogicalPath, $asset->logicalPath));
}
$queue[] = $javaScriptAsset;
}
$dependencies = array_merge($dependencies, $this->findEagerImports($nextAsset));
}

return $dependencies;
return array_keys($dependencies);
}

private function createMissingImportMapAssetException(ImportMapEntry $entry): \InvalidArgumentException
Expand Down
Expand Up @@ -671,6 +671,25 @@ public function getEagerEntrypointImportsTests(): iterable
['/assets/imports_simple.js', '/assets/simple.js'],
[$simpleAsset, $importsSimpleAsset],
];

$importsSimpleAsset2 = new MappedAsset(
'imports_simple2.js',
'/path/to/imports_simple2.js',
publicPathWithoutDigest: '/assets/imports_simple2.js',
javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false)]
);
yield 'an entry recursive dependencies are deduplicated' => [
new MappedAsset(
'app.js',
publicPath: '/assets/app.js',
javaScriptImports: [
new JavaScriptImport('/assets/imports_simple.js', assetLogicalPath: $importsSimpleAsset->logicalPath, assetSourcePath: $importsSimpleAsset->sourcePath, isLazy: false),
new JavaScriptImport('/assets/imports_simple2.js', assetLogicalPath: $importsSimpleAsset2->logicalPath, assetSourcePath: $importsSimpleAsset2->sourcePath, isLazy: false),
]
),
['/assets/imports_simple.js', '/assets/imports_simple2.js', '/assets/simple.js'],
[$simpleAsset, $importsSimpleAsset, $importsSimpleAsset2],
];
}

public function testFindEagerEntrypointImportsUsesCacheFile()
Expand Down

0 comments on commit 77189f6

Please sign in to comment.