8000 Merge branch '6.3' into 6.4 · symfony/symfony@bd57368 · GitHub
[go: up one dir, main page]
Skip to content

Commit bd57368

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.3' into 6.4
* 6.3: Update github/workflows/scorecard [AssetMapper] Avoid loading potentially ALL assets in dev server Fix CI for experimental mode [VarDumper] Fix `dd()` showing line with `null` [AssetMapper] Sometimes asset contents are built from non-asset files
2 parents 18ce05d + 27154d0 commit bd57368

21 files changed

+192
-26
lines changed

‎.github/workflows/scorecards.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ on:
66
schedule:
77
- cron: '34 4 * * 6'
88
push:
9-
branches: [ "6.3" ]
9+
branches: [ "6.4" ]
1010

1111
# Declare default permissions as read only.
1212
permissions: read-all

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
service('asset_mapper'),
8484
abstract_arg('asset public prefix'),
8585
abstract_arg('extensions map'),
86+
service('cache.asset_mapper'),
8687
])
8788
->tag('kernel.event_subscriber', ['event' => RequestEvent::class])
8889

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@
6666
->private()
6767
->tag('cache.pool')
6868

69+
->set('cache.asset_mapper')
70+
->parent('cache.system')
71+
->private()
72+
->tag('cache.pool')
73+
6974
->set('cache.messenger.restart_workers_signal')
7075
->parent('cache.app')
7176
->private()

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,15 @@ public function getAsset(string $logicalPath): ?MappedAsset
4444
return $this->mappedAssetFactory->createMappedAsset($logicalPath, $filePath);
4545
}
4646

47-
/**
48-
* @return MappedAsset[]
49-
*/
50-
public function allAssets(): array
47+
public function allAssets(): iterable
5148
{
52-
$assets = [];
5349
foreach ($this->mapperRepository->all() as $logicalPath => $filePath) {
5450
$asset = $this->getAsset($logicalPath);
5551
if (null === $asset) {
5652
throw new \LogicException(sprintf('Asset "%s" could not be found.', $logicalPath));
5753
}
58-
$assets[] = $asset;
54+
yield $asset;
5955
}
60-
61-
return $assets;
6256
}
6357

6458
public function getAssetFromSourcePath(string $sourcePath): ?MappedAsset

‎src/Symfony/Component/AssetMapper/AssetMapperDevServerSubscriber.php

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\AssetMapper;
1313

14+
use Psr\Cache\CacheItemPoolInterface;
1415
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -104,6 +105,7 @@ public function __construct(
104105
private readonly AssetMapperInterface $assetMapper,
105106
string $publicPrefix = '/assets/',
106107
array $extensionsMap = [],
108+
private readonly ?CacheItemPoolInterface $cacheMapCache = null,
107109
) {
108110
$this->publicPrefix = rtrim($publicPrefix, '/').'/';
109111
$this->extensionsMap = array_merge(self::EXTENSIONS_MAP, $extensionsMap);
@@ -120,13 +122,7 @@ public function onKernelRequest(RequestEvent $event): void
120122
return;
121123
}
122124

123-
$asset = null;
124-
foreach ($this->assetMapper->allAssets() as $assetCandidate) {
125-
if ($pathInfo === $assetCandidate->getPublicPath()) {
126-
$asset = $assetCandidate;
127-
break;
128-
}
129-
}
125+
$asset = $this->findAssetFromCache($pathInfo);
130126

131127
if (!$asset) {
132128
throw new NotFoundHttpException(sprintf('Asset with public path "%s" not found.', $pathInfo));
@@ -160,4 +156,37 @@ private function getMediaType(string $path): ?string
160156

161157
return $this->extensionsMap[$extension] ?? null;
162158
}
159+
160+
private function findAssetFromCache(string $pathInfo): ?MappedAsset
161+
{
162+
$cachedAsset = null;
163+
if (null !== $this->cacheMapCache) {
164+
$cachedAsset = $this->cacheMapCache->getItem(hash('xxh128', $pathInfo));
165+
$asset = $cachedAsset->isHit() ? $this->assetMapper->getAsset($cachedAsset->get()) : null;
166+
167+
if (null !== $asset && $asset->getPublicPath() === $pathInfo) {
168+
return $asset;
169+
}
170+
}
171+
172+
// we did not find a match
173+
$asset = null;
174+
foreach ($this->assetMapper->allAssets() as $assetCandidate) {
175+
if ($pathInfo === $assetCandidate->getPublicPath()) {
176+
$asset = $assetCandidate;
177+
break;
178+
}
179+
}
180+
181+
if (null === $asset) {
182+
return null;
183+
}
184+
185+
if (null !== $cachedAsset) {
186+
$cachedAsset->set($asset->getLogicalPath());
187+
$this->cacheMapCache->save($cachedAsset);
188+
}
189+
190+
return $asset;
191+
}
163192
}

‎src/Symfony/Component/AssetMapper/AssetMapperInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public function getAsset(string $logicalPath): ?MappedAsset;
2828
/**
2929
* Returns all mapped assets.
3030
*
31-
* @return MappedAsset[]
31+
* @return iterable<MappedAsset>
3232
*/
33-
public function allAssets(): array;
33+
public function allAssets(): iterable;
3434

3535
/**
3636
* Fetches the asset given its source path (i.e. filesystem path).

‎src/Symfony/Component/AssetMapper/Command/AssetMapperCompileCommand.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir
118118
{
119119
$allAssets = $this->assetMapper->allAssets();
120120

121-
$io->comment(sprintf('Compiling <info>%d</info> assets to <info>%s%s</info>', \count($allAssets), $publicDir, $this->publicAssetsPathResolver->resolvePublicPath('')));
121+
$io->comment(sprintf('Compiling assets to <info>%s%s</info>', $publicDir, $this->publicAssetsPathResolver->resolvePublicPath('')));
122122
$manifest = [];
123123
foreach ($allAssets as $asset) {
124124
// $asset->getPublicPath() will start with a "/"
@@ -132,6 +132,7 @@ private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir
132132
$manifest[$asset->getLogicalPath()] = $asset->getPublicPath();
133133
}
134134
ksort($manifest);
135+
$io->comment(sprintf('Compiled <info>%d</info> assets', \count($manifest)));
135136

136137
return $manifest;
137138
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ private function getCacheFilePath(string $logicalPath, string $sourcePath): stri
5959
*/
6060
private function collectResourcesFromAsset(MappedAsset $mappedAsset): array
6161
{
62-
$resources = [new FileResource($mappedAsset->getSourcePath())];
62+
$resources = array_map(fn (string $path) => new FileResource($path), $mappedAsset->getFileDependencies());
63+
$resources[] = new FileResource($mappedAsset->getSourcePath());
6364

6465
foreach ($mappedAsset->getDependencies() as $dependency) {
6566
if (!$dependency->isContentDependency) {

‎src/Symfony/Component/AssetMapper/MappedAsset.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ final class MappedAsset
3131
private bool $isPredigested;
3232
/** @var AssetDependency[] */
3333
private array $dependencies = [];
34+
/** @var string[] */
35+
private array $fileDependencies = [];
3436

3537
public function __construct(private readonly string $logicalPath)
3638
{
@@ -79,6 +81,14 @@ public function getDependencies(): array
7981
return $this->dependencies;
8082
}
8183

84+
/**
85+
* @return string[]
86+
*/
87+
public function getFileDependencies(): array
88+
{
89+
return $this->fileDependencies;
90+
}
91+
8292
public function setPublicPath(string $publicPath): void
8393
{
8494
if (isset($this->publicPath)) {
@@ -130,6 +140,16 @@ public function addDependency(AssetDependency $assetDependency): void
130140
$this->dependencies[] = $assetDependency;
131141
}
132142

143+
/**
144+
* Any filesystem files whose contents are used to create this asset.
145+
*
146+
* This is used to invalidate the cache when any of these files change.
147+
*/
148+
public function addFileDependency(string $sourcePath): void
149+
{
150+
$this->fileDependencies[] = $sourcePath;
151+
}
152+
133153
public function getPublicPathWithoutDigest(): string
134154
{
135155
return $this->publicPathWithoutDigest;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public function testAllAssets()
6969
});
7070

7171
$assets = $assetMapper->allAssets();
72+
$this->assertIsIterable($assets);
73+
$assets = iterator_to_array($assets);
7274
$this->assertCount(8, $assets);
7375
$this->assertInstanceOf(MappedAsset::class, $assets[0]);
7476
}

‎src/Symfony/Component/AssetMapper/Tests/Command/AssetsMapperCompileCommandTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function testAssetsAreCompiled()
5252
$res = $tester->execute([]);
5353
$this->assertSame(0, $res);
5454
// match Compiling \d+ assets
55-
$this->assertMatchesRegularExpression('/Compiling \d+ assets/', $tester->getDisplay());
55+
$this->assertMatchesRegularExpression('/Compiled \d+ assets/', $tester->getDisplay());
5656

5757
$this->assertFileExists($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js');
5858
$this->assertSame(<<<EOF

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ public function testAssetConfigCacheResourceContainsDependencies()
106106
$notDependentOnContentAsset->setSourcePath(__DIR__.'/../fixtures/dir2/already-abcdefVWXYZ0123456789.digested.css');
107107
$mappedAsset->addDependency(new AssetDependency($notDependentOnContentAsset, isContentDependency: false));
108108

109+
// just adding any file as an example
110+
$mappedAsset->addFileDependency(__DIR__.'/../fixtures/importmap.php');
111+
109112
$factory = $this->createMock(MappedAssetFactoryInterface::class);
110113
$factory->expects($this->once())
111114
->method('createMappedAsset')
@@ -119,12 +122,13 @@ public function testAssetConfigCacheResourceContainsDependencies()
119122
$cachedFactory->createMappedAsset('file1.css', $sourcePath);
120123

121124
$configCacheMetadata = $this->loadConfigCacheMetadataFor($mappedAsset);
122-
$this->assertCount(3, $configCacheMetadata);
125+
$this->assertCount(4, $configCacheMetadata);
123126
$this->assertInstanceOf(FileResource::class, $configCacheMetadata[0]);
124127
$this->assertInstanceOf(FileResource::class, $configCacheMetadata[1]);
125-
$this->assertSame($mappedAsset->getSourcePath(), $configCacheMetadata[0]->getResource());
126-
$this->assertSame($dependentOnContentAsset->getSourcePath(), $configCacheMetadata[1]->getResource());
127-
$this->assertSame($deeplyNestedAsset->getSourcePath(), $configCacheMetadata[2]->getResource());
128+
$this->assertSame(realpath(__DIR__.'/../fixtures/importmap.php'), $configCacheMetadata[0]->getResource());
129+
$this->assertSame($mappedAsset->getSourcePath(), $configCacheMetadata[1]->getResource());
130+
$this->assertSame($dependentOnContentAsset->getSourcePath(), $configCacheMetadata[2]->getResource());
131+
$this->assertSame($deeplyNestedAsset->getSourcePath(), $configCacheMetadata[3]->getResource());
128132
}
129133

130134
private function loadConfigCacheMetadataFor(MappedAsset $mappedAsset): array

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\AssetMapper\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\AssetMapper\AssetDependency;
1516
use Symfony\Component\AssetMapper\MappedAsset;
1617

1718
class MappedAssetTest extends TestCase
@@ -78,4 +79,17 @@ public function testGetContent()
7879
$asset->setContent('body { color: red; }');
7980
$this->assertSame('body { color: red; }', $asset->getContent());
8081
}
82+
83+
public function testAddDependencies()
84+
{
85+
$mainAsset = new MappedAsset('file.js');
86+
87+
$assetFoo = new MappedAsset('foo.js');
88+
$dependency = new AssetDependency($assetFoo, false, false);
89+
$mainAsset->addDependency($dependency);
90+
$mainAsset->addFileDependency('/path/to/foo.js');
91+
92+
$this->assertSame([$dependency], $mainAsset->getDependencies());
93+
$this->assertSame(['/path/to/foo.js'], $mainAsset->getFileDependencies());
94+
}
8195
}

‎src/Symfony/Component/VarDumper/Resources/functions/dump.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function dd(mixed ...$vars): never
4949
header('HTTP/1.1 500 Internal Server Error');
5050
}
5151

52-
if (isset($vars[0]) && 1 === count($vars)) {
52+
if (array_key_exists(0, $vars) && 1 === count($vars)) {
5353
VarDumper::dump($vars[0]);
5454
} else {
5555
foreach ($vars as $k => $v) {

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dd_with_multiple_args.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test dd() with multiple args shows line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dd(null, 1, 'foo');
13+
14+
--EXPECT--
15+
1 null
16+
2 1
17+
3 "foo"

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dd_with_null.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Test dd() with null doesn't show line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dd(null);
13+
14+
--EXPECT--
15+
null

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dd_with_single_arg.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Test dd() with one arg doesn't show line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dd('foo');
13+
14+
--EXPECT--
15+
"foo"

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dump_with_multiple_args.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test dump() with multiple args shows line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dump(null, 1, 'foo');
13+
14+
--EXPECT--
15+
1 null
16+
2 1
17+
3 "foo"

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dump_with_null.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Test dump() with null doesn't show line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dump(null);
13+
14+
--EXPECT--
15+
null

‎src/Symfony/Component/VarDumper/Tests/Dumper/functions/dump_with_single_arg.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Test dump() with one arg doesn't show line number
3+
--FILE--
4+
<?php
5+
6+
$vendor = __DIR__;
7+
while (!file_exists($vendor.'/vendor')) {
8+
$vendor = \dirname($vendor);
9+
}
10+
require $vendor.'/vendor/autoload.php';
11+
12+
dump('foo');
13+
14+
--EXPECT--
15+
"foo"

‎src/Symfony/Component/VarDumper/phpunit.xml.dist

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
<testsuites>
1818
<testsuite name="Symfony VarDumper Component Test Suite">
1919
<directory>./Tests/</directory>
20+
<directory suffix=".phpt">./Tests/Dumper/functions/</directory>
2021
</testsuite>
2122
</testsuites>
2223

0 commit comments

Comments
 (0)
0