bug #237 Fix missing integrity hash on preload tags (arnaud-ritti, Ko…

…cal) This PR was merged into the 2.x branch. Discussion ---------- Fix missing integrity hash on preload tags I wasn't able to edit #161, so I've opened a new PR and made changes here. Fix #101, fix #225 On an existing app, integrity hashes and other attributes are nicely injected into `Link` header: <img width="1101" alt="image" src="https://github.com/user-attachments/assets/3733d80f-1927-49d9-8f0e-c8e340f7ed69"> Commits ------- b8219a8 Pass all attributes to `Link` header 707cd49 style: php-cs-fixer 2025c41 Fix missing integrity hash on preload
symfony · Oct 2, 2024 · e335394 · e335394
2 parents 16af8a3 + b8219a8
commit e335394
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## v2.2.0
+
+- #236 Allow entrypoints.json to be hosted remotely (@rlvdx & @Kocal)
+- #232 fix: correctly wire the build-time file into kernel.build_dir (@dkarlovi)
+- #237 Fix missing integrity hash on preload (@arnaud-ritti & @Kocal)
+
 ## v2.1.0
 
 - #233 Add support for PHP 8.3 and PHP 8.4 (@Kocal)

diff --git a/src/Asset/TagRenderer.php b/src/Asset/TagRenderer.php
@@ -28,7 +28,10 @@ class TagRenderer implements ResetInterface
     private $defaultLinkAttributes;
     private $eventDispatcher;
 
+    // TODO WebpackEncoreBundle 3.0: remove this property
     private $renderedFiles = [];
+    // TODO WebpackEncoreBundle 3.0: rename this property to $renderedFiles
+    private $renderedFilesWithAttributes = [];
 
     public function __construct(
         EntrypointLookupCollectionInterface $entrypointLookupCollection,
@@ -48,7 +51,7 @@ public function __construct(
         $this->reset();
     }
 
-    public function renderWebpackScriptTags(string $entryName, ?string $packageName = null, ?string $entrypointName = null, array $extraAttributes = []): string
+    public function renderWebpackScriptTags(string $entryName, ?string $packageName = null, ?string $entrypointName = null, array $extraAttributes = [], bool $includeAttributes = false): string
     {
         $entrypointName = $entrypointName ?: '_default';
         $scriptTags = [];
@@ -80,6 +83,7 @@ public function renderWebpackScriptTags(string $entryName, ?string $packageName
             );
 
             $this->renderedFiles['scripts'][] = $attributes['src'];
+            $this->renderedFilesWithAttributes['scripts'][] = $attributes;
         }
 
         return implode('', $scriptTags);
@@ -118,19 +122,36 @@ public function renderWebpackLinkTags(string $entryName, ?string $packageName =
             );
 
             $this->renderedFiles['styles'][] = $attributes['href'];
+            $this->renderedFilesWithAttributes['styles'][] = $attributes;
         }
 
         return implode('', $scriptTags);
     }
 
-    public function getRenderedScripts(): array
+    /**
+     * @param bool $includeAttributes Whether to include the attributes or not.
+     *                                In WebpackEncoreBundle 3.0, this parameter will be removed,
+     *                                and the attributes will always be included.
+     *                                TODO WebpackEncoreBundle 3.0
+     *
+     * @return ($includeAttributes is true ? list<array<string, mixed>> : list<string>)
+     */
+    public function getRenderedScripts(bool $includeAttributes = false): array
     {
-        return $this->renderedFiles['scripts'];
+        return $includeAttributes ? $this->renderedFilesWithAttributes['scripts'] : $this->renderedFiles['scripts'];
     }
 
-    public function getRenderedStyles(): array
+    /**
+     * @param bool $includeAttributes Whether to include the attributes or not.
+     *                                In WebpackEncoreBundle 3.0, this parameter will be removed,
+     *                                and the attributes will always be included.
+     *                                TODO WebpackEncoreBundle 3.0
+     *
+     * @return ($includeAttributes is true ? list<array<string, mixed>> : list<string>)
+     */
+    public function getRenderedStyles(bool $includeAttributes = false): array
     {
-        return $this->renderedFiles['styles'];
+        return $includeAttributes ? $this->renderedFilesWithAttributes['styles'] : $this->renderedFiles['styles'];
     }
 
     public function getDefaultAttributes(): array
@@ -140,7 +161,7 @@ public function getDefaultAttributes(): array
 
     public function reset(): void
     {
-        $this->renderedFiles = [
+        $this->renderedFiles = $this->renderedFilesWithAttributes = [
             'scripts' => [],
             'styles' => [],
         ];

diff --git a/src/EventListener/PreLoadAssetsEventListener.php b/src/EventListener/PreLoadAssetsEventListener.php
@@ -47,23 +47,31 @@ public function onKernelResponse(ResponseEvent $event): void
         /** @var GenericLinkProvider $linkProvider */
         $linkProvider = $request->attributes->get('_links');
         $defaultAttributes = $this->tagRenderer->getDefaultAttributes();
-        $crossOrigin = $defaultAttributes['crossorigin'] ?? false;
 
-        foreach ($this->tagRenderer->getRenderedScripts() as $href) {
-            $link = $this->createLink('preload', $href)->withAttribute('as', 'script');
+        foreach ($this->tagRenderer->getRenderedScripts(true) as $attributes) {
+            $src = $attributes['src'];
+            unset($attributes['src']);
+            $attributes = [...$defaultAttributes, ...$attributes];
 
-            if (false !== $crossOrigin) {
-                $link = $link->withAttribute('crossorigin', $crossOrigin);
+            $link = $this->createLink('preload', $src)
+                ->withAttribute('as', 'script');
+
+            foreach ($attributes as $k => $v) {
+                $link = $link->withAttribute($k, $v);
             }
 
             $linkProvider = $linkProvider->withLink($link);
         }
 
-        foreach ($this->tagRenderer->getRenderedStyles() as $href) {
+        foreach ($this->tagRenderer->getRenderedStyles(true) as $attributes) {
+            $href = $attributes['href'];
+            unset($attributes['href']);
+            $attributes = [...$defaultAttributes, ...$attributes];
+
             $link = $this->createLink('preload', $href)->withAttribute('as', 'style');
 
-            if (false !== $crossOrigin) {
-                $link = $link->withAttribute('crossorigin', $crossOrigin);
+            foreach ($attributes as $k => $v) {
+                $link = $link->withAttribute($k, $v);
             }
 
             $linkProvider = $linkProvider->withLink($link);

diff --git a/tests/Asset/TagRendererTest.php b/tests/Asset/TagRendererTest.php
@@ -301,8 +301,25 @@ public function testGetRenderedFilesAndReset()
         $this->assertSame(['http://localhost:8080/build/file1.js', 'http://localhost:8080/build/file2.js'], $renderer->getRenderedScripts());
         $this->assertSame(['http://localhost:8080/build/file1.css'], $renderer->getRenderedStyles());
 
+        $this->assertSame([
+            [
+                'src' => 'http://localhost:8080/build/file1.js',
+            ],
+            [
+                'src' => 'http://localhost:8080/build/file2.js',
+            ],
+        ], $renderer->getRenderedScripts(true));
+        $this->assertSame([
+            [
+                'rel' => 'stylesheet',
+                'href' => 'http://localhost:8080/build/file1.css',
+            ],
+        ], $renderer->getRenderedStyles(true));
+
         $renderer->reset();
         $this->assertEmpty($renderer->getRenderedScripts());
         $this->assertEmpty($renderer->getRenderedStyles());
+        $this->assertEmpty($renderer->getRenderedScripts(true));
+        $this->assertEmpty($renderer->getRenderedStyles(true));
     }
 }
diff --git a/tests/EventListener/PreLoadAssetsEventListenerTest.php b/tests/EventListener/PreLoadAssetsEventListenerTest.php
@@ -30,8 +30,17 @@ public function testItPreloadsAssets()
     {
         $tagRenderer = $this->createMock(TagRenderer::class);
         $tagRenderer->expects($this->once())->method('getDefaultAttributes')->willReturn(['crossorigin' => 'anonymous']);
-        $tagRenderer->expects($this->once())->method('getRenderedScripts')->willReturn(['/file1.js']);
-        $tagRenderer->expects($this->once())->method('getRenderedStyles')->willReturn(['/css/file1.css']);
+        $tagRenderer->expects($this->once())->method('getRenderedScripts')->with(true)->willReturn([
+            [
+                'src' => '/file1.js',
+            ],
+        ]);
+        $tagRenderer->expects($this->once())->method('getRenderedStyles')->with(true)->willReturn([
+            [
+                'rel' => 'stylesheet',
+                'href' => '/css/file1.css',
+            ],
+        ]);
 
         $request = new Request();
         $response = new Response();
@@ -53,14 +62,18 @@ public function testItPreloadsAssets()
 
         $this->assertSame('/css/file1.css', $links[1]->getHref());
         $this->assertSame(['preload'], $links[1]->getRels());
-        $this->assertSame(['as' => 'style', 'crossorigin' => 'anonymous'], $links[1]->getAttributes());
+        $this->assertSame(['as' => 'style', 'crossorigin' => 'anonymous', 'rel' => 'stylesheet'], $links[1]->getAttributes());
     }
 
     public function testItReusesExistingLinkProvider()
     {
         $tagRenderer = $this->createMock(TagRenderer::class);
         $tagRenderer->expects($this->once())->method('getDefaultAttributes')->willReturn(['crossorigin' => 'anonymous']);
-        $tagRenderer->expects($this->once())->method('getRenderedScripts')->willReturn(['/file1.js']);
+        $tagRenderer->expects($this->once())->method('getRenderedScripts')->willReturn([
+            [
+                'src' => '/file1.js',
+            ],
+        ]);
         $tagRenderer->expects($this->once())->method('getRenderedStyles')->willReturn([]);
 
         $request = new Request();