8000 feature #51786 [AssetMapper] Always downloading vendor files (weaverr… · symfony/symfony@204381b · GitHub
[go: up one dir, main page]
Skip to content

Commit 204381b

Browse files
fabpotfabpot
committed
feature #51786 [AssetMapper] Always downloading vendor files (weaverryan)
This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [AssetMapper] Always downloading vendor files | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | yes | Deprecations? | no | Tickets | None | License | MIT Hi! I discovered a few problems with our CDN set up: A) The `jsdelivr` CDN uses imports statements with specific versions - e.g. `import{Controller as t}from"/npm/`@hotwired`/stimulus@3.2.1/+esm";`. This could cause the same dependency to be downloaded (at different verisons) every time. B) The `jspm` CDN has correct bare imports - e.g. `import '`@hotwired`/stimulus';`. However, to they also include some "in-package" relative imports - e.g.`import '../other-file.js'`. That is ok on the CDN, but to support local download, it means we ALSO need to download those files. This is possible, but adds quite a bit of complexity and would probably require an `importmap.php.local` file so we could keep track of all of the files needed for some `foo-bar` package. ### Solution Talking with `@nicolas`-grekas, we're going to change AssetMapper to "download-only" - like Composer, npm, etc. The `importmap.php` file will change to look like: ```php return [ // no change for local files 'app' => [ 'path' => './assets/app.js', 'entrypoint' => true, ], // remote packages have only version 'bootstrap' => [ 'version' => '5.3.2', ], ``` We will exclusively use `jsdelivr` as the "backend", as it avoids the "in-package" relative imports. So, every package == 1 file. To download the remote packages, run: ``` php bin/console importmap:install ``` This downloads everything into an `assets/vendor/` directory (which will be ignored by git). ### Other Notable Items * Added `MappedAsset.isVendor` property, which is a simple check to see if the file lives in `assets/vendor/`. If it does, we still run the compilers, but we can silence invalid import warnings. * Mapped assets ending in `.php` are excluded from AssetMapper. There could be edge-cases where you don't want this, but it felt like a safer default, and allowed me to easily NOT expose the new `vendor/assets/installed.php` file. TODO * [x] Re-add code to change downloaded contents to bare imports * [x] Download vendor packages after update/require * [x] In JsCompiler, don't warn on vendor invalid imports * [x] Remove package resolver system * [x] Fix tests * [x] Audit to see what tests are missing Commits ------- 18e4200 [AssetMapper] Always downloading vendor files
2 parents 95fa158 + 18e4200 commit 204381b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+867
-1013
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ CHANGELOG
2626
* Deprecate `framework.validation.enable_annotations`, use `framework.validation.enable_attributes` instead
2727
* Deprecate `framework.serializer.enable_annotations`, use `framework.serializer.enable_attributes` instead
2828
* Add `array $tokenAttributes = []` optional parameter to `KernelBrowser::loginUser()`
29-
* Add support for relative URLs in BrowserKit's redirect assertion.
29+
* Add support for relative URLs in BrowserKit's redirect assertion
3030
* Change BrowserKitAssertionsTrait::getClient() to be protected
31+
* Deprecate the `framework.asset_mapper.provider` config option
3132

3233
6.3
3334
---

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ class UnusedTagsPass implements CompilerPassInterface
2525
'annotations.cached_reader',
2626
'assets.package',
2727
'asset_mapper.compiler',
28-
'asset_mapper.importmap.resolver',
2928
'auto_alias',
3029
'cache.pool',
3130
'cache.pool.clearer',

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use Symfony\Bundle\FullStack;
1919
use Symfony\Component\Asset\Package;
2020
use Symfony\Component\AssetMapper\AssetMapper;
21-
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
2221
use Symfony\Component\Cache\Adapter\DoctrineAdapter;
2322
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
2423
use Symfony\Component\Config\Definition\Builder\NodeBuilder;
@@ -940,8 +939,7 @@ private function addAssetMapperSection(ArrayNodeDefinition $rootNode, callable $
940939
->defaultValue('%kernel.project_dir%/assets/vendor')
941940
->end()
942941
->scalarNode('provider')
943-
->info('The provider (CDN) to use'.(class_exists(ImportMapManager::class) ? sprintf(' (e.g.: "%s").', implode('", "', ImportMapManager::PROVIDERS)) : '.'))
944-
->defaultValue('jsdelivr.esm')
942+
->setDeprecated('symfony/framework-bundle', '6.4', 'Option "%node%" at "%path%" is deprecated and does nothing. Remove it.')
945943
->end()
946944
->end()
947945
->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
use Symfony\Component\AssetMapper\AssetMapper;
3535
use Symfony\Component\AssetMapper\Compiler\AssetCompilerInterface;
3636
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
37-
use Symfony\Component\AssetMapper\ImportMap\Resolver\PackageResolverInterface;
3837
use Symfony\Component\BrowserKit\AbstractBrowser;
3938
use Symfony\Component\Cache\Adapter\AdapterInterface;
4039
use Symfony\Component\Cache\Adapter\ArrayAdapter;
@@ -1364,28 +1363,24 @@ private function registerAssetMapperConfiguration(array $config, ContainerBuilde
13641363
->setArgument(1, $config['missing_import_mode']);
13651364

13661365
$container
1367-
->getDefinition('asset_mapper.importmap.manager')
1368-
->replaceArgument(3, $config['vendor_dir'])
1366+
->getDefinition('asset_mapper.importmap.remote_package_downloader')
1367+
->replaceArgument(2, $config['vendor_dir'])
13691368
;
1370-
13711369
$container
1372-
->getDefinition('asset_mapper.importmap.config_reader')
1373-
->replaceArgument(0, $config['importmap_path'])
1370+
->getDefinition('asset_mapper.mapped_asset_factory')
1371+
->replaceArgument(2, $config['vendor_dir'])
13741372
;
13751373

13761374
$container
1377-
->getDefinition('asset_mapper.importmap.resolver')
1378-
->replaceArgument(0, $config['provider'])
1375+
->getDefinition('asset_mapper.importmap.config_reader')
1376+
->replaceArgument(0, $config['importmap_path'])
13791377
;
13801378

13811379
$container
13821380
->getDefinition('asset_mapper.importmap.renderer')
13831381
->replaceArgument(3, $config['importmap_polyfill'] ?? ImportMapManager::POLYFILL_URL)
13841382
->replaceArgument(4, $config['importmap_script_attributes'])
13851383
;
1386-
1387-
$container->registerForAutoconfiguration(PackageResolverInterface::class)
1388-
->addTag('asset_mapper.importmap.resolver');
13891384
}
13901385

13911386
/**

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

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@
3232
use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader;
3333
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
3434
use Symfony\Component\AssetMapper\ImportMap\ImportMapRenderer;
35+
use Symfony\Component\AssetMapper\ImportMap\RemotePackageDownloader;
3536
use Symfony\Component\AssetMapper\ImportMap\Resolver\JsDelivrEsmResolver;
36-
use Symfony\Component\AssetMapper\ImportMap\Resolver\JspmResolver;
37-
use Symfony\Component\AssetMapper\ImportMap\Resolver\PackageResolver;
3837
use Symfony\Component\AssetMapper\MapperAwareAssetPackage;
3938
use Symfony\Component\AssetMapper\Path\PublicAssetsPathResolver;
4039
use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -53,6 +52,7 @@
5352
->args([
5453
service('asset_mapper.public_assets_path_resolver'),
5554
service('asset_mapper_compiler'),
55+
abstract_arg('vendor directory'),
5656
])
5757

5858
->set('asset_mapper.cached_mapped_asset_factory', CachedMappedAssetFactory::class)
@@ -150,41 +150,20 @@
150150
service('asset_mapper'),
151151
service('asset_mapper.public_assets_path_resolver'),
152152
service('asset_mapper.importmap.config_reader'),
153-
abstract_arg('vendor directory'),
153+
service('asset_mapper.importmap.remote_package_downloader'),
154154
service('asset_mapper.importmap.resolver'),
155-
service('http_client'),
156155
])
157156
->alias(ImportMapManager::class, 'asset_mapper.importmap.manager')
158157

159-
->set('asset_mapper.importmap.resolver', PackageResolver::class)
158+
->set('asset_mapper.importmap.remote_package_downloader', RemotePackageDownloader::class)
160159
->args([
161-
abstract_arg('provider'),
162-
tagged_locator('asset_mapper.importmap.resolver'),
160+
service('asset_mapper.importmap.config_reader'),
161+
service('asset_mapper.importmap.resolver'),
162+
abstract_arg('vendor directory'),
163163
])
164164

165-
->set('asset_mapper.importmap.resolver.jsdelivr_esm', JsDelivrEsmResolver::class)
165+
->set('asset_mapper.importmap.resolver', JsDelivrEsmResolver::class)
166166
->args([service('http_client')])
167-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_JSDELIVR_ESM])
168-
169-
->set('asset_mapper.importmap.resolver.jspm', JspmResolver::class)
170-
->args([service('http_client'), ImportMapManager::PROVIDER_JSPM])
171-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_JSPM])
172-
173-
->set('asset_mapper.importmap.resolver.jspm_system', JspmResolver::class)
174-
->args([service('http_client'), ImportMapManager::PROVIDER_JSPM_SYSTEM])
175-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_JSPM_SYSTEM])
176-
177-
->set('asset_mapper.importmap.resolver.skypack', JspmResolver::class)
178-
->args([service('http_client'), ImportMapManager::PROVIDER_SKYPACK])
179-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_SKYPACK])
180-
181-
->set('asset_mapper.importmap.resolver.jsdelivr', JspmResolver::class)
182-
->args([service('http_client'), ImportMapManager::PROVIDER_JSDELIVR])
183-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_JSDELIVR])
184-
185-
->set('asset_mapper.importmap.resolver.unpkg', JspmResolver::class)
186-
->args([service('http_client'), ImportMapManager::PROVIDER_UNPKG])
187-
->tag('asset_mapper.importmap.resolver', ['resolver' => ImportMapManager::PROVIDER_UNPKG])
188167

189168
->set('asset_mapper.importmap.renderer', ImportMapRenderer::class)
190169
->args([
@@ -198,14 +177,12 @@
198177
->set('asset_mapper.importmap.auditor', ImportMapAuditor::class)
199178
->args([
200179
service('asset_mapper.importmap.config_reader'),
201-
service('asset_mapper.importmap.resolver'),
202180
service('http_client'),
203181
])
204182

205183
->set('asset_mapper.importmap.command.require', ImportMapRequireCommand::class)
206184
->args([
207185
service('asset_mapper.importmap.manager'),
208-
service('asset_mapper'),
209186
param('kernel.project_dir'),
210187
])
211188
->tag('console.command')
@@ -219,7 +196,10 @@
219196
->tag('console.command')
220197

221198
->set('asset_mapper.importmap.command.install', ImportMapInstallCommand::class)
222-
->args([service('asset_mapper.importmap.manager')])
199+
->args([
200+
service('asset_mapper.importmap.remote_package_downloader'),
201+
param('kernel.project_dir'),
202+
])
223203
->tag('console.command')
224204

225205
->set('asset_mapper.importmap.command.audit', ImportMapAuditCommand::class)

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ public function testAssetMapperCanBeEnabled()
134134
'importmap_path' => '%kernel.project_dir%/importmap.php',
135135
'importmap_polyfill' => null,
136136
'vendor_dir' => '%kernel.project_dir%/assets/vendor',
137-
'provider' => 'jsdelivr.esm',
138137
'importmap_script_attributes' => [],
139138
];
140139

@@ -671,7 +670,6 @@ protected static function getBundleDefaultConfig()
671670
'importmap_path' => '%kernel.project_dir%/importmap.php',
672671
'importmap_polyfill' => null,
673672
'vendor_dir' => '%kernel.project_dir%/assets/vendor',
674-
'provider' => 'jsdelivr.esm',
675673
'importmap_script_attributes' => [],
676674
],
677675
'cache' => [

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/asset_mapper.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
importmap-path="%kernel.project_dir%/importmap.php"
1818
importmap-polyfill="https://cdn.example.com/polyfill.js"
1919
vendor-dir="%kernel.project_dir%/assets/vendor"
20-
provider="jspm"
2120
>
2221
<framework:path>assets/</framework:path>
2322
<framework:path namespace="my_namespace">assets2/</framework:path>

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ public function all(): array
103103
foreach ($this->getDirectories() as $path => $namespace) {
104104
$iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($path));
105105
foreach ($iterator as $file) {
106+
/** @var \SplFileInfo $file */
106107
if (!$file->isFile()) {
107108
continue;
108109
}
@@ -111,6 +112,11 @@ public function all(): array
111112
continue;
112113
}
113114

115+
// avoid potentially exposing PHP files
116+
if ('php' === $file->getExtension()) {
117+
continue;
118+
}
119+
114120
/** @var RecursiveDirectoryIterator $innerIterator */
115121
$innerIterator = $iterator->getInnerIterator();
116122
$logicalPath = ($namespace ? rtrim($namespace, '/').'/' : '').$innerIterator->getSubPathName();

‎src/Symfony/Component/AssetMapper/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ CHANGELOG
77
* Mark the component as non experimental
88
* Add CSS support to the importmap
99
* Add "entrypoints" concept to the importmap
10+
* Always download packages locally instead of using a CDN
1011
* Allow relative path strings in the importmap
1112
* Add `PreAssetsCompileEvent` event when running `asset-map:compile`
1213
* Add support for importmap paths to use the Asset component (for subdirectories)

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@
1111

1212
namespace Symfony\Component\AssetMapper\Command;
1313

14-
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
14+
use Symfony\Component\AssetMapper\ImportMap\RemotePackageDownloader;
1515
use Symfony\Component\Console\Attribute\AsCommand;
1616
use Symfony\Component\Console\Command\Command;
17+
use Symfony\Component\Console\Helper\ProgressBar;
1718
use Symfony\Component\Console\Input\InputInterface;
1819
use Symfony\Component\Console\Output\OutputInterface;
1920
use Symfony\Component\Console\Style\SymfonyStyle;
21+
use Symfony\Contracts\HttpClient\ResponseInterface;
2022

2123
/**
2224
* Downloads all assets that should be downloaded.
@@ -27,7 +29,8 @@
2729
final class ImportMapInstallCommand extends Command
2830
{
2931
public function __construct(
30-
private readonly ImportMapManager $importMapManager,
32+
private readonly RemotePackageDownloader $packageDownloader,
33+
private readonly string $projectDir,
3134
) {
3235
parent::__construct();
3336
}
@@ -36,8 +39,36 @@ protected function execute(InputInterface $input, OutputInterface $output): int
3639
{
3740
$io = new SymfonyStyle($input, $output);
3841

39-
$downloadedPackages = $this->importMapManager->downloadMissingPackages();
40-
$io->success(sprintf('Downloaded %d assets.', \count($downloadedPackages)));
42+
$finishedCount = 0;
43+
$progressBar = new ProgressBar($output);
44+
$progressBar->setFormat('<info>%current%/%max%</info> %bar% %url%');
45+
$downloadedPackages = $this->packageDownloader->downloadPackages(function (string $package, string $event, ResponseInterface $response, int $totalPackages) use (&$finishedCount, $progressBar) {
46+
$progressBar->setMessage($response->getInfo('url'), 'url');
47+
if (0 === $progressBar->getMaxSteps()) {
48+
$progressBar->setMaxSteps($totalPackages);
49+
$progressBar->start();
50+
}
51+
52+
if ('finished' === $event) {
53+
++$finishedCount;
54+
$progressBar->advance();
55+
}
56+
});
57+
$progressBar->finish();
58+
$progressBar->clear();
59+
60+
if (!$downloadedPackages) {
61+
$io->success('No assets to install.');
62+
63+
return Command::SUCCESS;
64+
}
65+
66+
$io->success(sprintf(
67+
'Downloaded %d asset%s into %s.',
68+
\count($downloadedPackages),
69+
1 === \count($downloadedPackages) ? '' : 's',
70+
str_replace($this->projectDir.'/', '', $this->packageDownloader->getVendorDir()),
71+
));
4172

4273
return Command::SUCCESS;
4374
}

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

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
namespace Symfony\Component\AssetMapper\Command;
1313

14-
use Symfony\Bundle\FrameworkBundle\Console\Application;
15-
use Symfony\Component\AssetMapper\AssetMapperInterface;
1614
use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry;
1715
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
1816
use Symfony\Component\AssetMapper\ImportMap\PackageRequireOptions;
@@ -32,7 +30,6 @@ final class ImportMapRequireCommand extends Command
3230
{
3331
public function __construct(
3432
private readonly ImportMapManager $importMapManager,
35-
private readonly AssetMapperInterface $assetMapper,
3633
private readonly string $projectDir,
3734
) {
3835
parent::__construct();
@@ -42,7 +39,7 @@ protected function configure(): void
4239
{
4340
$this
4441
->addArgument('packages', InputArgument::IS_ARRAY | InputArgument::REQUIRED, 'The packages to add')
45-
->addOption('download', 'd', InputOption::VALUE_NONE, 'Download packages locally')
42+
->addOption('entrypoint', null, InputOption::VALUE_NONE, 'Make the package(s) an entrypoint?')
4643
->addOption('path', null, InputOption::VALUE_REQUIRED, 'The local path where the package lives relative to the project root')
4744
->setHelp(<<<'EOT'
4845
The <info>%command.name%</info> command adds packages to <comment>importmap.php</comment> usually
@@ -113,10 +110,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
113110
$packages[] = new PackageRequireOptions(
114111
$parts['package'],
115112
$parts['version'] ?? null,
116-
$input->getOption('download'),
117113
$parts['alias'] ?? $parts['package'],
118-
isset($parts['registry']) && $parts['registry'] ? $parts['registry'] : null,
119114
$path,
115+
$input->getOption('entrypoint'),
120116
);
121117
}
122118

@@ -125,19 +121,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
125121
$newPackage = $newPackages[0];
126122
$message = sprintf('Package "%s" added to importmap.php', $newPackage->importName);
127123

128-
if ($newPackage->isDownloaded && null !== $downloadedAsset = $this->assetMapper->getAsset($newPackage->path)) {
129-
$application = $this->getApplication();
130-
if ($application instanceof Application) {
131-
$projectDir = $application->getKernel()->getProjectDir();
132-
$downloadedPath = $downloadedAsset->sourcePath;
133-
if (str_starts_with($downloadedPath, $projectDir)) {
134-
$downloadedPath = substr($downloadedPath, \strlen($projectDir) + 1);
135-
}
136-
137-
$message .= sprintf(' and downloaded locally to "%s"', $downloadedPath);
138-
}
139-
}
140-
141124
$message .= '.';
142125
} else {
143126
$names = array_map(fn (ImportMapEntry $package) => $package->importName, $newPackages);

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 + E963 146,7 @@ private function findAssetForBareImport(string $importedModule, AssetMapperInter
146146
}
147147

148148
// remote entries have no MappedAsset
149-
if ($importMapEntry->isRemote()) {
149+
if ($importMapEntry->isRemotePackage()) {
150150
return null;
151151
}
152152

@@ -158,7 +158,10 @@ private function findAssetForRelativeImport(string $importedModule, MappedAsset
158158
try {
159159
$resolvedPath = $this->resolvePath(\dirname($asset->logicalPath), $importedModule);
160160
} catch (RuntimeException $e) {
161-
$this->handleMissingImport(sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);
161+
// avoid warning about vendor imports - these are often comments
162+
if (!$asset->isVendor) {
163+
$this->handleMissingImport(sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);
164+
}
162165

163166
return null;
164167
}
@@ -179,7 +182,10 @@ private function findAssetForRelativeImport(string $importedModule, MappedAsset
179182
// avoid circular error if there is self-referencing import comments
180183
}
181184

182-
$this->handleMissingImport($message);
185+
// avoid warning about vendor imports - these are often comments
186+
if (!$asset->isVendor) {
187+
$this->handleMissingImport($message);
188+
}
183189

184190
return null;
185191
}

0 commit comments

Comments
 (0)
0