Skip to content

Commit

Permalink
bug #51880 [AssetMapper] Fix entrypoint scripts are not preloaded (sm…
Browse files Browse the repository at this point in the history
…nandre)

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

Discussion
----------

[AssetMapper] Fix entrypoint scripts are not preloaded

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

Preload the "rendered" importmap entryoints (ex: "app.js")
(expected behaviour)

Simple test with "`app.js`" entrypoint and two imported scripts "`foo.js`" and ... "`Crocodile.js`".

**Before** (priority: normal / order: last)

![before](https://github.com/symfony/symfony/assets/1359581/cabde9da-e790-44f5-9ea6-317d994ef57b)

**After** (expected behaviour: highest priority)

![after](https://github.com/symfony/symfony/assets/1359581/88a781a1-015a-42bb-9eb6-df581112c21e)

(screenshots made with Firefox on macOS)

Commits
-------

769b896 [AssetMapper] Fix entrypoint scripts are not preloaded
  • Loading branch information
fabpot committed Oct 7, 2023
2 parents b4224ce + 769b896 commit d8397de
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 11 deletions.
21 changes: 10 additions & 11 deletions src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,22 @@ public function getImportMapData(array $entrypointNames): array
{
$rawImportMapData = $this->getRawImportMapData();
$finalImportMapData = [];
foreach ($entrypointNames as $entry) {
$finalImportMapData[$entry] = $rawImportMapData[$entry];
foreach ($this->findEagerEntrypointImports($entry) as $dependency) {
if (isset($finalImportMapData[$dependency])) {
foreach ($entrypointNames as $entrypointName) {
$entrypointImports = $this->findEagerEntrypointImports($entrypointName);
// Entrypoint modules must be preloaded before their dependencies
foreach ([$entrypointName, ...$entrypointImports] as $import) {
if (isset($finalImportMapData[$import])) {
continue;
}

if (!isset($rawImportMapData[$dependency])) {
// missing dependency - rely on browser or compilers to warn
// Missing dependency - rely on browser or compilers to warn
if (!isset($rawImportMapData[$import])) {
continue;
}

// re-order the final array by order of dependencies
$finalImportMapData[$dependency] = $rawImportMapData[$dependency];
// and mark for preloading
$finalImportMapData[$dependency]['preload'] = true;
unset($rawImportMapData[$dependency]);
$finalImportMapData[$import] = $rawImportMapData[$import];
$finalImportMapData[$import]['preload'] = true;
unset($rawImportMapData[$import]);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ public function testGetImportMapData()
path: 'entry2.js',
isEntrypoint: true,
),
new ImportMapEntry(
'entry3',
path: 'entry3.js',
isEntrypoint: true,
),
new ImportMapEntry(
'normal_js_file',
path: 'normal_js_file.js',
Expand All @@ -483,6 +488,11 @@ public function testGetImportMapData()
publicPathWithoutDigest: '/assets/imported_file2.js',
publicPath: '/assets/imported_file2-d1g35t.js',
);
$importedFile3 = new MappedAsset(
'imported_file3.js',
publicPathWithoutDigest: '/assets/imported_file3.js',
publicPath: '/assets/imported_file3-d1g35t.js',
);
$normalJsFile = new MappedAsset(
'normal_js_file.js',
publicPathWithoutDigest: '/assets/normal_js_file.js',
Expand Down Expand Up @@ -527,8 +537,16 @@ public function testGetImportMapData()
new JavaScriptImport('/assets/styles/file2.css', isLazy: false, asset: $importedCss2, addImplicitlyToImportMap: true),
]
),
new MappedAsset(
'entry3.js',
publicPath: '/assets/entry3-d1g35t.js',
javaScriptImports: [
new JavaScriptImport('/assets/imported_file3.js', isLazy: false, asset: $importedFile3),
],
),
$importedFile1,
$importedFile2,
// $importedFile3,
$normalJsFile,
$importedCss1,
$importedCss2,
Expand All @@ -542,6 +560,7 @@ public function testGetImportMapData()
'entry1' => [
'path' => '/assets/entry1-d1g35t.js',
'type' => 'js',
'preload' => true, // Rendered entry points are preloaded
],
'/assets/imported_file1.js' => [
'path' => '/assets/imported_file1-d1g35t.js',
Expand All @@ -551,6 +570,7 @@ public function testGetImportMapData()
'entry2' => [
'path' => '/assets/entry2-d1g35t.js',
'type' => 'js',
'preload' => true, // Rendered entry points are preloaded
],
'/assets/imported_file2.js' => [
'path' => '/assets/imported_file2-d1g35t.js',
Expand All @@ -577,6 +597,10 @@ public function testGetImportMapData()
'type' => 'css',
'preload' => true,
],
'entry3' => [
'path' => '/assets/entry3-d1g35t.js',
'type' => 'js', // No preload (entry point not "rendered")
],
'never_imported_css' => [
'path' => '/assets/styles/never_imported_css-d1g35t.css',
'type' => 'css',
Expand All @@ -598,6 +622,7 @@ public function testGetImportMapData()
'normal_js_file',

// importmap entries never imported
'entry3',
'never_imported_css',
], array_keys($actualImportMapData));
}
Expand Down

0 comments on commit d8397de

Please sign in to comment.