diff --git a/src/Flex.php b/src/Flex.php index 6c53270a..713d2a8f 100644 --- a/src/Flex.php +++ b/src/Flex.php @@ -32,6 +32,7 @@ use Composer\IO\NullIO; use Composer\Json\JsonFile; use Composer\Json\JsonManipulator; +use Composer\Package\CompletePackageInterface; use Composer\Package\Locker; use Composer\Package\Package; use Composer\Plugin\PluginEvents; @@ -76,6 +77,7 @@ class Flex implements PluginInterface, EventSubscriberInterface private $installer; private $postInstallOutput = ['']; private $operations = []; + private $uninstalledImportMapEntryNames = []; private $lock; private $displayThanksReminder = 0; private $ignorePreleases = false; @@ -327,6 +329,24 @@ public function record(PackageEvent $event) } } + public function recordUninstalledImportMapEntryNames(PackageEvent $event): void + { + $operation = $event->getOperation(); + if (!$operation instanceof UninstallOperation) { + return; + } + + // the importmap entries of the package must be resolved now, while its files still exist + $package = $operation->getPackage(); + $synchronizer = $this->createPackageJsonSynchronizer($this->options->get('root-dir')); + $entryNames = $synchronizer->resolveImportMapEntryNames([ + 'name' => $package->getName(), + 'keywords' => $package instanceof CompletePackageInterface ? ($package->getKeywords() ?: []) : [], + ]); + + $this->uninstalledImportMapEntryNames = array_merge($this->uninstalledImportMapEntryNames, $entryNames); + } + public function recordOperations(InstallerEvent $event) { if (!$event->isExecutingOperations()) { @@ -566,23 +586,29 @@ private function synchronizePackageJson(string $rootDir) return; } - $rootDir = realpath($rootDir); - $vendorDir = trim((new Filesystem())->makePathRelative($this->config->get('vendor-dir'), $rootDir), '/'); - - $executor = new ScriptExecutor($this->composer, $this->io, $this->options); - $synchronizer = new PackageJsonSynchronizer($rootDir, $vendorDir, $executor, $this->io); + $synchronizer = $this->createPackageJsonSynchronizer($rootDir); if ($synchronizer->shouldSynchronize()) { $lockData = $this->composer->getLocker()->getLockData(); - if ($synchronizer->synchronize(array_merge($lockData['packages'] ?? [], $lockData['packages-dev'] ?? []))) { + if ($synchronizer->synchronize(array_merge($lockData['packages'] ?? [], $lockData['packages-dev'] ?? []), $this->uninstalledImportMapEntryNames)) { $this->io->writeError('Synchronizing package.json with PHP packages'); $this->io->writeError('Don\'t forget to run npm install --force or yarn install --force to refresh your JavaScript dependencies!'); $this->io->writeError(''); } + + $this->uninstalledImportMapEntryNames = []; } } + private function createPackageJsonSynchronizer(string $rootDir): PackageJsonSynchronizer + { + $rootDir = realpath($rootDir); + $vendorDir = trim((new Filesystem())->makePathRelative($this->config->get('vendor-dir'), $rootDir), '/'); + + return new PackageJsonSynchronizer($rootDir, $vendorDir, new ScriptExecutor($this->composer, $this->io, $this->options), $this->io); + } + /** * @return void */ @@ -899,6 +925,7 @@ public static function getSubscribedEvents(): array $events = [ PackageEvents::POST_PACKAGE_UPDATE => 'enableThanksReminder', PackageEvents::POST_PACKAGE_INSTALL => 'recordFlexInstall', + PackageEvents::PRE_PACKAGE_UNINSTALL => 'recordUninstalledImportMapEntryNames', PackageEvents::POST_PACKAGE_UNINSTALL => 'record', InstallerEvents::PRE_OPERATIONS_EXEC => 'recordOperations', PluginEvents::PRE_POOL_CREATE => 'truncatePackages', diff --git a/src/PackageJsonSynchronizer.php b/src/PackageJsonSynchronizer.php index 5daec3ba..edccecc5 100644 --- a/src/PackageJsonSynchronizer.php +++ b/src/PackageJsonSynchronizer.php @@ -44,10 +44,14 @@ public function shouldSynchronize(): bool return $this->rootDir && (file_exists($this->rootDir.'/package.json') || file_exists($this->rootDir.'/importmap.php')); } - public function synchronize(array $phpPackages): bool + /** + * @param string[] $obsoleteImportMapEntryNames importmap entry names declared by packages + * that have just been uninstalled + */ + public function synchronize(array $phpPackages, array $obsoleteImportMapEntryNames = []): bool { if (file_exists($this->rootDir.'/importmap.php')) { - $this->synchronizeForAssetMapper($phpPackages); + $this->synchronizeForAssetMapper($phpPackages, $obsoleteImportMapEntryNames); return false; } @@ -78,7 +82,24 @@ public function synchronize(array $phpPackages): bool return $didChangePackageJson; } - private function synchronizeForAssetMapper(array $phpPackages): void + /** + * Returns the importmap entry names declared by a PHP package. + * + * Useful to capture the entries of a package about to be uninstalled, while + * its files still exist. + * + * @return string[] + */ + public function resolveImportMapEntryNames(array $phpPackage): array + { + if (!$packageJson = $this->resolvePackageJson($phpPackage)) { + return []; + } + + return array_keys($packageJson->read()['symfony']['importmap'] ?? []); + } + + private function synchronizeForAssetMapper(array $phpPackages, array $obsoleteImportMapEntryNames): void { $importMapEntries = []; $phpPackages = $this->normalizePhpPackages($phpPackages); @@ -88,6 +109,7 @@ private function synchronizeForAssetMapper(array $phpPackages): void } } + $this->removeObsoleteImportMapEntries($obsoleteImportMapEntryNames, $importMapEntries); $this->updateImportMap($importMapEntries); $this->updateControllersJsonFile($phpPackages); } @@ -254,6 +276,38 @@ private function shouldUpdateConstraint(string $existingConstraint, string $cons } } + /** + * @param string[] $obsoleteImportMapEntryNames + * @param array $keptImportMapEntries + */ + private function removeObsoleteImportMapEntries(array $obsoleteImportMapEntryNames, array $keptImportMapEntries): void + { + if (!$obsoleteImportMapEntryNames) { + return; + } + + $importMapData = include $this->rootDir.'/importmap.php'; + + $toRemove = []; + foreach (array_unique($obsoleteImportMapEntryNames) as $name) { + // the entry is still declared by an installed package + if (isset($keptImportMapEntries[$name])) { + continue; + } + + // the entry is not in the importmap (e.g. already removed by hand) + if (!isset($importMapData[$name])) { + continue; + } + + $toRemove[] = $name; + } + + if ($toRemove) { + $this->scriptExecutor->execute('symfony-cmd', 'importmap:remove', $toRemove); + } + } + /** * @param array $importMapEntries */ diff --git a/tests/PackageJsonSynchronizerTest.php b/tests/PackageJsonSynchronizerTest.php index 9970e73d..f5997de3 100644 --- a/tests/PackageJsonSynchronizerTest.php +++ b/tests/PackageJsonSynchronizerTest.php @@ -501,4 +501,97 @@ public function testExceptionWhenInvalidImportMapConstraint() ], ]); } + + public function testSynchronizeAssetMapperRemovesObsoleteImportMapEntries() + { + $importMap = [ + 'app' => [ + 'path' => './assets/app.js', + 'entrypoint' => true, + ], + '@hotcake/foo' => [ + // constraint in package.json is ^1.9.0 + 'version' => '1.9.1', + ], + '@removed/package' => [ + 'version' => '3.0.0', + ], + '@removed/package/script.js' => [ + 'path' => './vendor/removed/package/assets/script.js', + ], + '@symfony/new-package' => [ + 'path' => './vendor/symfony/new-package/assets/dist/loader.js', + ], + '@symfony/new-package/entry.js' => [ + 'path' => './vendor/symfony/new-package/assets/entry.js', + 'entrypoint' => true, + ], + '@symfony/new-package/entry2.js' => [ + 'path' => './vendor/symfony/new-package/assets/entry2.js', + 'entrypoint' => true, + ], + ]; + file_put_contents($this->tempDir.'/importmap.php', \sprintf('scriptExecutor->expects($this->once()) + ->method('execute') + ->willReturnCallback(function (...$arguments) use (&$actualArguments) { $actualArguments[] = $arguments; }); + + $this->synchronizer->synchronize( + [ + [ + 'name' => 'symfony/new-package', + 'keywords' => ['symfony-ux'], + ], + ], + [ + // obsolete, must be removed + '@removed/package', + '@removed/package/script.js', + // still declared by symfony/new-package, must be kept + '@hotcake/foo', + // not in the importmap (e.g. already removed by hand), must be ignored + '@removed/package/other.js', + ] + ); + + $this->assertSame( + [ + ['symfony-cmd', 'importmap:remove', ['@removed/package', '@removed/package/script.js']], + ], + $actualArguments + ); + } + + public function testResolveImportMapEntryNames() + { + $this->scriptExecutor->expects($this->never())->method('execute'); + + $this->assertSame( + [ + '@hotcake/foo', + '@symfony/new-package', + '@symfony/new-package/entry.js', + '@symfony/new-package/entry2.js', + ], + $this->synchronizer->resolveImportMapEntryNames([ + 'name' => 'symfony/new-package', + 'keywords' => ['symfony-ux'], + ]) + ); + + // package without the "symfony-ux" keyword + $this->assertSame([], $this->synchronizer->resolveImportMapEntryNames([ + 'name' => 'symfony/new-package', + 'keywords' => [], + ])); + + // package without importmap config + $this->assertSame([], $this->synchronizer->resolveImportMapEntryNames([ + 'name' => 'symfony/existing-package', + 'keywords' => ['symfony-ux'], + ])); + } + }