feature symfony#59464 [AssetMapper] Add --dry-run option on importmap:require command (chadyred)

fabpot · fabpot · commit c11abf2b5385 · 2025-03-10T15:39:30.000-07:00
This PR was merged into the 7.3 branch. Discussion ---------- [AssetMapper] Add `--dry-run` option on `importmap:require` command | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT We are working with `@smnandre` on several ideas to **improve AssetMapper DX**, focusing here on the `importmap:require` command. Currently, there is no way to know in advance which dependent packages will be downloaded too, and the final output display lacks clarity. This PR aims to solve both issues by adding a new `--dry-run` flag and reworking the command output (with `-v` verbosity) <img width="845" alt="1" src="https://github.com/user-attachments/assets/26616802-f6f1-413f-9535-7a1a5205d277" /> <details><summary>Console output</summary> ```console php bin/console importmap:require bootstrap --dry-run -v [DRY-RUN] No changes will apply to the importmap configuration. -------------------------------------- --------- ---------------------------------------------------- Package Version Path -------------------------------------- --------- ---------------------------------------------------- bootstrap 5.3.3 assets/vendor/bootstrap/bootstrap.index.js `@popperjs`/core 2.11.8 assets/vendor/`@popperjs`/core/core.index.js bootstrap/dist/css/bootstrap.min.css 5.3.3 assets/vendor/bootstrap/dist/css/bootstrap.min.css -------------------------------------- --------- ---------------------------------------------------- [OK] 3 new items (bootstrap, `@popperjs`/core, bootstrap/dist/css/bootstrap.min.css) added to the importmap.php! [DRY-RUN] No changes applied to the importmap configuration. ``` </details> ## New `--dry-run` option This flag allows developers to simulate the installation process, without any alteration: - no modification of `importmap.php` file; - no file added or updated in the `assets` and `public` directories. The `--dry-run` flag allows developers to: - **check** commands for **mistakes** (e.g., typing `bootrap` instead of `bootstrap`, or `jquery`); - **list** dependent **packages** that will be installed (e.g., requiring `bootstrap` will also install ``@popperjs`/core`); - **test** for potential issues while **downloading**[^1] files. The console will display a line at start and end of execution to signal the dry-run mode. <img width="845" alt="2" src="https://github.com/user-attachments/assets/4df6abc0-6205-410e-9039-8c4b37a41055" /> <details><summary>Console output</summary> ```bash php bin/console importmap:require bootstrap --dry-run [DRY-RUN] No changes will apply to the importmap configuration. [OK] 3 new items (bootstrap, `@popperjs`/core, bootstrap/dist/css/bootstrap.min.css) added to the importmap.php! [DRY-RUN] No changes applied to the importmap configuration. ``` </details> ## New `-v` verbose output This PR also reworks the output for the `-v` (verbose) flag to provide more clarity when running the command with additional verbosity. <img width="845" alt="3" src="https://github.com/user-attachments/assets/46ff8d22-887c-4900-b778-36d9aaac1512" /> --- All feedback welcome! [^1]: AssetMapper recursively extracts the dependencies of JavaScript modules. Even with the `--dry-run` option, the `importmap:require` command requires a working `HttpClient` to download metadata and JavaScript files from various registries/repositories (mainly JSDelivr for now). Commits ------- 337c465 [AssetMapper] Add `--dry-run` option on `importmap:require` command
diff --git a/UPGRADE-7.3.md b/UPGRADE-7.3.md
@@ -191,3 +191,8 @@ VarDumper
 
  * Deprecate `ResourceCaster::castCurl()`, `ResourceCaster::castGd()` and `ResourceCaster::castOpensslX509()`
  * Mark all casters as `@internal`
+
+AssetMapper
+-----------
+
+ * `ImportMapRequireCommand` now takes `projectDir` as a required third constructor argument
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php
@@ -232,6 +232,7 @@
             ->args([
                 service('asset_mapper.importmap.manager'),
                 service('asset_mapper.importmap.version_checker'),
+                param('kernel.project_dir'),
             ])
             ->tag('console.command')
 
diff --git a/src/Symfony/Component/AssetMapper/CHANGELOG.md b/src/Symfony/Component/AssetMapper/CHANGELOG.md
@@ -5,6 +5,8 @@ CHANGELOG
 ---
 
  * Add support for pre-compressing assets with Brotli, Zstandard, Zopfli, and gzip
+ * Add option `--dry-run` to `importmap:require` command
+ * `ImportMapRequireCommand` now takes `projectDir` as a required third constructor argument
 
 7.2
 ---
diff --git a/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php b/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\AssetMapper\Command;
 
+use Symfony\Component\AssetMapper\ImportMap\ImportMapEntries;
 use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry;
 use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
 use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker;
@@ -22,6 +23,7 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\Filesystem\Path;
 
 /**
  * @author Kévin Dunglas <kevin@dunglas.dev>
@@ -34,16 +36,22 @@ final class ImportMapRequireCommand extends Command
     public function __construct(
         private readonly ImportMapManager $importMapManager,
         private readonly ImportMapVersionChecker $importMapVersionChecker,
+        private readonly ?string $projectDir = null,
     ) {
+        if (null === $projectDir) {
+            trigger_deprecation('symfony/asset-mapper', '7.3', 'The "%s()" method will have a new `string $projectDir` argument in version 8.0, not defining it is deprecated.', __METHOD__);
+        }
+
         parent::__construct();
     }
 
     protected function configure(): void
     {
         $this
             ->addArgument('packages', InputArgument::IS_ARRAY | InputArgument::REQUIRED, 'The packages to add')
-            ->addOption('entrypoint', null, InputOption::VALUE_NONE, 'Make the package(s) an entrypoint?')
+            ->addOption('entrypoint', null, InputOption::VALUE_NONE, 'Make the packages an entrypoint?')
             ->addOption('path', null, InputOption::VALUE_REQUIRED, 'The local path where the package lives relative to the project root')
+            ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Simulate the installation of the packages')
             ->setHelp(<<<'EOT'
 The <info>%command.name%</info> command adds packages to <comment>importmap.php</comment> usually
 by finding a CDN URL for the given package and version.
@@ -72,6 +80,11 @@ protected function configure(): void
 
     <info>php %command.full_name% "any_module_name" --path=./assets/some_file.js</info>
 
+To simulate the installation, use the <info>--dry-run</info> option:
+
+    <info>php %command.full_name% "any_module_name" --dry-run -v</info>
+
+When this option is enabled, this command does not perform any write operations to the filesystem.
 EOT
             );
     }
@@ -92,6 +105,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $path = $input->getOption('path');
         }
 
+        if ($input->getOption('dry-run')) {
+            $io->writeln(['', '<comment>[DRY-RUN]</comment> No changes will apply to the importmap configuration.', '']);
+        }
+
         $packages = [];
         foreach ($packageList as $packageName) {
             $parts = ImportMapManager::parsePackageName($packageName);
@@ -110,28 +127,45 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             );
         }
 
-        $newPackages = $this->importMapManager->require($packages);
+        if ($input->getOption('dry-run')) {
+            $newPackages = $this->importMapManager->requirePackages($packages, new ImportMapEntries());
+        } else {
+            $newPackages = $this->importMapManager->require($packages);
+        }
 
         $this->renderVersionProblems($this->importMapVersionChecker, $output);
 
-        if (1 === \count($newPackages)) {
-            $newPackage = $newPackages[0];
-            $message = \sprintf('Package "%s" added to importmap.php', $newPackage->importName);
+        $newPackageNames = array_map(fn (ImportMapEntry $package): string => $package->importName, $newPackages);
 
-            $message .= '.';
+        if (1 === \count($newPackages)) {
+            $messages = [\sprintf('Package "%s" added to importmap.php.', $newPackageNames[0])];
         } else {
-            $names = array_map(fn (ImportMapEntry $package) => $package->importName, $newPackages);
-            $message = \sprintf('%d new items (%s) added to the importmap.php!', \count($newPackages), implode(', ', $names));
+            $messages = [\sprintf('%d new items (%s) added to the importmap.php!', \count($newPackages), implode(', ', $newPackageNames))];
         }
 
-        $messages = [$message];
+        if ($io->isVerbose()) {
+            $io->table(
+                ['Package', 'Version', 'Path'],
+                array_map(fn (ImportMapEntry $package): array => [
+                    $package->importName,
+                    $package->version ?? '-',
+                    // BC layer for AssetMapper < 7.3
+                    // When `projectDir` is not null, we use the absolute path of the package
+                    null !== $this->projectDir ? Path::makeRelative($package->path, $this->projectDir) : $package->path,
+                ], $newPackages),
+            );
+        }
 
         if (1 === \count($newPackages)) {
             $messages[] = \sprintf('Use the new package normally by importing "%s".', $newPackages[0]->importName);
         }
 
         $io->success($messages);
 
+        if ($input->getOption('dry-run')) {
+            $io->writeln(['<comment>[DRY-RUN]</comment> No changes applied to the importmap configuration.', '']);
+        }
+
         return Command::SUCCESS;
     }
 }
diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapManager.php
@@ -128,13 +128,15 @@ private function updateImportMapConfig(bool $update, array $packagesToRequire, a
     }
 
     /**
+     * @internal
+     *
      * Gets information about (and optionally downloads) the packages & updates the entries.
      *
      * Returns an array of the entries that were added.
      *
      * @param PackageRequireOptions[] $packagesToRequire
      */
-    private function requirePackages(array $packagesToRequire, ImportMapEntries $importMapEntries): array
+    public function requirePackages(array $packagesToRequire, ImportMapEntries $importMapEntries): array
     {
         if (!$packagesToRequire) {
             return [];
diff --git a/src/Symfony/Component/AssetMapper/Tests/Command/ImportMapRequireCommandTest.php b/src/Symfony/Component/AssetMapper/Tests/Command/ImportMapRequireCommandTest.php
@@ -0,0 +1,218 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\AssetMapper\Tests\Command;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symfony\Bundle\FrameworkBundle\Console\Application;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\AssetMapper\Command\ImportMapRequireCommand;
+use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry;
+use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
+use Symfony\Component\AssetMapper\ImportMap\ImportMapType;
+use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker;
+use Symfony\Component\AssetMapper\Tests\Fixtures\ImportMapTestAppKernel;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Tester\CommandTester;
+use Symfony\Component\Filesystem\Filesystem;
+use Symfony\Component\Finder\Finder;
+
+class ImportMapRequireCommandTest extends KernelTestCase
+{
+    protected static function getKernelClass(): string
+    {
+        return ImportMapTestAppKernel::class;
+    }
+
+    /**
+     * @dataProvider getRequirePackageTests
+     */
+    public function testDryRunOptionToShowInformationBeforeApplyInstallation(int $verbosity, array $packageEntries, array $packagesToInstall, string $expected, ?string $path = null)
+    {
+        $importMapManager = $this->createMock(ImportMapManager::class);
+        $importMapManager
+            ->method('requirePackages')
+            ->willReturn($packageEntries)
+        ;
+
+        $command = new ImportMapRequireCommand(
+            $importMapManager,
+            $this->createMock(ImportMapVersionChecker::class),
+            '/path/to/project/dir',
+        );
+
+        $args = [
+            'packages' => $packagesToInstall,
+            '--dry-run' => true,
+        ];
+        if ($path) {
+            $args['--path'] = $path;
+        }
+
+        $commandTester = new CommandTester($command);
+        $commandTester->execute($args, ['verbosity' => $verbosity]);
+
+        $commandTester->assertCommandIsSuccessful();
+
+        $output = $commandTester->getDisplay();
+        $this->assertEquals($this->trimBeginEndOfEachLine($expected), $this->trimBeginEndOfEachLine($output));
+    }
+
+    public static function getRequirePackageTests(): iterable
+    {
+        yield 'require package with dry run and normal verbosity options' => [
+            OutputInterface::VERBOSITY_NORMAL,
+            [self::createRemoteEntry('bootstrap', '4.2.3', 'assets/vendor/bootstrap/bootstrap.js')],
+            ['bootstrap'], <<<EOF
+ [DRY-RUN] No changes will apply to the importmap configuration.
+
+ [OK] Package "bootstrap" added to importmap.php.
+
+ Use the new package normally by importing "bootstrap".
+
+ [DRY-RUN] No changes applied to the importmap configuration.
+ EOF,
+        ];
+
+        yield 'remote package requested with a version with dry run and verbosity verbose options' => [
+            OutputInterface::VERBOSITY_VERBOSE,
+            [self::createRemoteEntry('bootstrap', '5.3.3', 'assets/vendor/bootstrap/bootstrap.js')],
+            ['bootstrap'], <<<EOF
+ [DRY-RUN] No changes will apply to the importmap configuration.
+
+ ----------- --------- --------------------------------------
+  Package     Version   Path
+ ----------- --------- --------------------------------------
+  bootstrap   5.3.3     assets/vendor/bootstrap/bootstrap.js
+ ----------- --------- --------------------------------------
+
+ [OK] Package "bootstrap" added to importmap.php.
+
+ Use the new package normally by importing "bootstrap".
+
+ [DRY-RUN] No changes applied to the importmap configuration.
+ EOF,
+        ];
+
+        yield 'local package require a path, with dry run and verbosity verbose options' => [
+            OutputInterface::VERBOSITY_VERBOSE,
+            [ImportMapEntry::createLocal('alice.js', ImportMapType::JS, 'assets/js/alice.js', false)],
+            ['alice.js'], <<<EOF
+ [DRY-RUN] No changes will apply to the importmap configuration.
+
+ ---------- --------- --------------------
+  Package    Version   Path
+ ---------- --------- --------------------
+  alice.js   -         assets/js/alice.js
+ ---------- --------- --------------------
+
+ [OK] Package "alice.js" added to importmap.php.
+
+ Use the new package normally by importing "alice.js".
+
+ [DRY-RUN] No changes applied to the importmap configuration.
+EOF,
+            './assets/alice.js',
+        ];
+
+        yield 'multi remote packages requested with dry run and verbosity normal options' => [
+            OutputInterface::VERBOSITY_NORMAL, [
+                self::createRemoteEntry('bootstrap', '5.3.3', 'assets/vendor/bootstrap/bootstrap.index.js'),
+                self::createRemoteEntry('lodash', '4.17.20', 'assets/vendor/lodash/lodash.index.js'),
+            ],
+            ['bootstrap lodash@4.17.21'], <<<EOF
+ [DRY-RUN] No changes will apply to the importmap configuration.
+
+ [OK] 2 new items (bootstrap, lodash) added to the importmap.php!
+
+ [DRY-RUN] No changes applied to the importmap configuration.
+ EOF,
+        ];
+
+        yield 'multi remote packages requested with dry run and verbosity verbose options' => [
+            OutputInterface::VERBOSITY_VERBOSE, [
+                self::createRemoteEntry('bootstrap', '5.3.3', 'assets/vendor/bootstrap/bootstrap.js'),
+                self::createRemoteEntry('lodash', '4.17.20', 'assets/vendor/lodash/lodash.index.js'),
+            ],
+            ['bootstrap lodash@4.17.21'], <<<EOF
+  [DRY-RUN] No changes will apply to the importmap configuration.
+
+ ----------- --------- --------------------------------------
+  Package     Version   Path
+ ----------- --------- --------------------------------------
+  bootstrap   5.3.3     assets/vendor/bootstrap/bootstrap.js
+  lodash      4.17.20   assets/vendor/lodash/lodash.index.js
+ ----------- --------- --------------------------------------
+
+ [OK] 2 new items (bootstrap, lodash) added to the importmap.php!
+
+ [DRY-RUN] No changes applied to the importmap configuration.
+ EOF,
+        ];
+    }
+
+    public function testNothingChangedOnFilesystemAfterUsingDryRunOption()
+    {
+        $kernel = self::bootKernel();
+        $projectDir = $kernel->getProjectDir();
+
+        $fs = new Filesystem();
+        $fs->mkdir($projectDir.'/public');
+
+        $fs->dumpFile($projectDir.'/public/assets/manifest.json', '{}');
+        $fs->dumpFile($projectDir.'/public/assets/importmap.json', '{}');
+
+        $importMapManager = $this->createMock(ImportMapManager::class);
+        $importMapManager
+            ->expects($this->once())
+            ->method('requirePackages')
+            ->willReturn([self::createRemoteEntry('bootstrap', '5.3.3', 'assets/vendor/bootstrap/bootstrap.index.js')]);
+
+        self::getContainer()->set(ImportMapManager::class, $importMapManager);
+
+        $application = new Application(self::$kernel);
+        $command = $application->find('importmap:require');
+
+        $importMapContentBefore = $fs->readFile($projectDir.'/importmap.php');
+        $installedVendorBefore = $fs->readFile($projectDir.'/assets/vendor/installed.php');
+
+        $tester = new CommandTester($command);
+        $tester->execute(['packages' => ['bootstrap'], '--dry-run' => true]);
+
+        $tester->assertCommandIsSuccessful();
+
+        $this->assertSame($importMapContentBefore, $fs->readFile($projectDir.'/importmap.php'));
+        $this->assertSame($installedVendorBefore, $fs->readFile($projectDir.'/assets/vendor/installed.php'));
+
+        $this->assertSame('{}', $fs->readFile($projectDir.'/public/assets/manifest.json'));
+        $this->assertSame('{}', $fs->readFile($projectDir.'/public/assets/importmap.json'));
+
+        $finder = new Finder();
+        $finder->in($projectDir.'/public/assets')->files()->depth(0);
+
+        $this->assertCount(2, $finder); // manifest.json + importmap.json
+
+        $fs->remove($projectDir.'/public');
+        $fs->remove($projectDir.'/var');
+
+        static::$kernel->shutdown();
+    }
+
+    private static function createRemoteEntry(string $importName, string $version, ?string $path = null): ImportMapEntry
+    {
+        return ImportMapEntry::createRemote($importName, ImportMapType::JS, path: $path, version: $version, packageModuleSpecifier: $importName, isEntrypoint: false);
+    }
+
+    private function trimBeginEndOfEachLine(string $lines): string
+    {
+        return trim(implode("\n", array_map('trim', explode("\n", $lines))));
+    }
+}
diff --git a/src/Symfony/Component/AssetMapper/Tests/Fixtures/AssetMapperTestAppKernel.php b/src/Symfony/Component/AssetMapper/Tests/Fixtures/AssetMapperTestAppKernel.php
@@ -44,7 +44,7 @@ public function registerContainerConfiguration(LoaderInterface $loader): void
                 'assets' => null,
                 'asset_mapper' => [
                     'paths' => ['dir1', 'dir2', 'non_ascii', 'assets'],
-                    'public_prefix' => 'assets'
+                    'public_prefix' => 'assets',
                 ],
                 'test' => true,
             ]);
diff --git a/src/Symfony/Component/AssetMapper/Tests/Fixtures/ImportMapTestAppKernel.php b/src/Symfony/Component/AssetMapper/Tests/Fixtures/ImportMapTestAppKernel.php

Original file line number	Diff line number	Diff line change
`@@ -128,13 +128,15 @@ private function updateImportMapConfig(bool $update, array $packagesToRequire, a`
`128`	`128`	`}`
`129`	`129`
`130`	`130`	`/**`
	`131`	`+ * @internal`
	`132`	`+ *`
`131`	`133`	`* Gets information about (and optionally downloads) the packages & updates the entries.`
`132`	`134`	`*`
`133`	`135`	`* Returns an array of the entries that were added.`
`134`	`136`	`*`
`135`	`137`	`* @param PackageRequireOptions[] $packagesToRequire`
`136`	`138`	`*/`
`137`		`- private function requirePackages(array $packagesToRequire, ImportMapEntries $importMapEntries): array`
	`139`	`+ public function requirePackages(array $packagesToRequire, ImportMapEntries $importMapEntries): array`
`138`	`140`	`{`
`139`	`141`	`if (!$packagesToRequire) {`
`140`	`142`	`return [];`