From 42dfb9aa1cf23c37bf4cd1740cbde557e9f25a42 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 4 Oct 2023 14:42:36 -0400 Subject: [PATCH] [AssetMapper] Warn of missing or incompat dependencies --- composer.json | 1 + .../Resources/config/asset_mapper.php | 13 +- .../Command/ImportMapRequireCommand.php | 7 + .../Command/ImportMapUpdateCommand.php | 8 +- .../Command/VersionProblemCommandTrait.php | 41 ++ .../ImportMap/ImportMapVersionChecker.php | 179 +++++++ .../ImportMap/PackageVersionProblem.php | 23 + .../ImportMap/RemotePackageDownloader.php | 22 +- .../Resolver/JsDelivrEsmResolver.php | 18 +- .../Resolver/PackageResolverInterface.php | 4 +- .../ImportMap/ImportMapVersionCheckerTest.php | 436 ++++++++++++++++++ .../ImportMap/RemotePackageDownloaderTest.php | 32 +- .../Resolver/JsDelivrEsmResolverTest.php | 45 +- .../fixtures/assets/vendor/installed.php | 2 + .../Component/AssetMapper/composer.json | 1 + 15 files changed, 795 insertions(+), 37 deletions(-) create mode 100644 src/Symfony/Component/AssetMapper/Command/VersionProblemCommandTrait.php create mode 100644 src/Symfony/Component/AssetMapper/ImportMap/ImportMapVersionChecker.php create mode 100644 src/Symfony/Component/AssetMapper/ImportMap/PackageVersionProblem.php create mode 100644 src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapVersionCheckerTest.php diff --git a/composer.json b/composer.json index 2808ca1211e9f..6fb094c569fa8 100644 --- a/composer.json +++ b/composer.json @@ -35,6 +35,7 @@ "require": { "php": ">=8.1", "composer-runtime-api": ">=2.1", + "composer/semver": "^3.0", "ext-xml": "*", "friendsofphp/proxy-manager-lts": "^1.0.2", "doctrine/event-manager": "^1.2|^2", diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php index d02bc2724d7ba..d31573031c656 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php @@ -34,6 +34,7 @@ use Symfony\Component\AssetMapper\ImportMap\ImportMapManager; use Symfony\Component\AssetMapper\ImportMap\ImportMapRenderer; use Symfony\Component\AssetMapper\ImportMap\ImportMapUpdateChecker; +use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker; use Symfony\Component\AssetMapper\ImportMap\RemotePackageDownloader; use Symfony\Component\AssetMapper\ImportMap\RemotePackageStorage; use Symfony\Component\AssetMapper\ImportMap\Resolver\JsDelivrEsmResolver; @@ -171,6 +172,12 @@ service('asset_mapper.importmap.resolver'), ]) + ->set('asset_mapper.importmap.version_checker', ImportMapVersionChecker::class) + ->args([ + service('asset_mapper.importmap.config_reader'), + service('asset_mapper.importmap.remote_package_downloader'), + ]) + ->set('asset_mapper.importmap.resolver', JsDelivrEsmResolver::class) ->args([service('http_client')]) @@ -199,6 +206,7 @@ ->args([ service('asset_mapper.importmap.manager'), param('kernel.project_dir'), + service('asset_mapper.importmap.version_checker'), ]) ->tag('console.command') @@ -207,7 +215,10 @@ ->tag('console.command') ->set('asset_mapper.importmap.command.update', ImportMapUpdateCommand::class) - ->args([service('asset_mapper.importmap.manager')]) + ->args([ + service('asset_mapper.importmap.manager'), + service('asset_mapper.importmap.version_checker'), + ]) ->tag('console.command') ->set('asset_mapper.importmap.command.install', ImportMapInstallCommand::class) diff --git a/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php b/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php index 6a5fb54e2781a..a7402ee92020a 100644 --- a/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php +++ b/src/Symfony/Component/AssetMapper/Command/ImportMapRequireCommand.php @@ -13,6 +13,7 @@ use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry; use Symfony\Component\AssetMapper\ImportMap\ImportMapManager; +use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker; use Symfony\Component\AssetMapper\ImportMap\PackageRequireOptions; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -28,9 +29,12 @@ #[AsCommand(name: 'importmap:require', description: 'Require JavaScript packages')] final class ImportMapRequireCommand extends Command { + use VersionProblemCommandTrait; + public function __construct( private readonly ImportMapManager $importMapManager, private readonly string $projectDir, + private readonly ImportMapVersionChecker $importMapVersionChecker, ) { parent::__construct(); } @@ -108,6 +112,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $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); diff --git a/src/Symfony/Component/AssetMapper/Command/ImportMapUpdateCommand.php b/src/Symfony/Component/AssetMapper/Command/ImportMapUpdateCommand.php index 86dc3fd896833..2c3c615f9a599 100644 --- a/src/Symfony/Component/AssetMapper/Command/ImportMapUpdateCommand.php +++ b/src/Symfony/Component/AssetMapper/Command/ImportMapUpdateCommand.php @@ -13,6 +13,7 @@ use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry; use Symfony\Component\AssetMapper\ImportMap\ImportMapManager; +use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -26,8 +27,11 @@ #[AsCommand(name: 'importmap:update', description: 'Update JavaScript packages to their latest versions')] final class ImportMapUpdateCommand extends Command { + use VersionProblemCommandTrait; + public function __construct( - protected readonly ImportMapManager $importMapManager, + private readonly ImportMapManager $importMapManager, + private readonly ImportMapVersionChecker $importMapVersionChecker, ) { parent::__construct(); } @@ -57,6 +61,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $io = new SymfonyStyle($input, $output); $updatedPackages = $this->importMapManager->update($packages); + $this->renderVersionProblems($this->importMapVersionChecker, $output); + if (0 < \count($packages)) { $io->success(sprintf( 'Updated %s package%s in importmap.php.', diff --git a/src/Symfony/Component/AssetMapper/Command/VersionProblemCommandTrait.php b/src/Symfony/Component/AssetMapper/Command/VersionProblemCommandTrait.php new file mode 100644 index 0000000000000..7a1b8f631332c --- /dev/null +++ b/src/Symfony/Component/AssetMapper/Command/VersionProblemCommandTrait.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\AssetMapper\Command; + +use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker; +use Symfony\Component\Console\Output\OutputInterface; + +/** + * @internal + */ +trait VersionProblemCommandTrait +{ + private function renderVersionProblems(ImportMapVersionChecker $importMapVersionChecker, OutputInterface $output): void + { + $problems = $importMapVersionChecker->checkVersions(); + foreach ($problems as $problem) { + if (null === $problem->installedVersion) { + $output->writeln(sprintf('[warning] %s requires %s but it is not in the importmap.php. You may need to run "php bin/console importmap:require %s".', $problem->packageName, $problem->dependencyPackageName, $problem->dependencyPackageName)); + + continue; + } + + if (null === $problem->requiredVersionConstraint) { + $output->writeln(sprintf('[warning] %s appears to import %s but this is not listed as a dependency of %s. This is odd and could be a misconfiguration of that package.', $problem->packageName, $problem->dependencyPackageName, $problem->packageName)); + + continue; + } + + $output->writeln(sprintf('[warning] %s requires %s@%s but version %s is installed.', $problem->packageName, $problem->dependencyPackageName, $problem->requiredVersionConstraint, $problem->installedVersion)); + } + } +} diff --git a/src/Symfony/Component/AssetMapper/ImportMap/ImportMapVersionChecker.php b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapVersionChecker.php new file mode 100644 index 0000000000000..5e7d8500a9417 --- /dev/null +++ b/src/Symfony/Component/AssetMapper/ImportMap/ImportMapVersionChecker.php @@ -0,0 +1,179 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\AssetMapper\ImportMap; + +use Composer\Semver\Semver; +use Symfony\Component\AssetMapper\Exception\RuntimeException; +use Symfony\Component\HttpClient\HttpClient; +use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface; +use Symfony\Contracts\HttpClient\HttpClientInterface; + +class ImportMapVersionChecker +{ + private const PACKAGE_METADATA_PATTERN = 'https://registry.npmjs.org/%package%/%version%'; + + private HttpClientInterface $httpClient; + + public function __construct( + private ImportMapConfigReader $importMapConfigReader, + private RemotePackageDownloader $packageDownloader, + HttpClientInterface $httpClient = null, + ) { + $this->httpClient = $httpClient ?? HttpClient::create(); + } + + /** + * @return PackageVersionProblem[] + */ + public function checkVersions(): array + { + $entries = $this->importMapConfigReader->getEntries(); + + $packages = []; + foreach ($entries as $entry) { + if (!$entry->isRemotePackage()) { + continue; + } + + $dependencies = $this->packageDownloader->getDependencies($entry->importName); + if (!$dependencies) { + continue; + } + + $packageName = $entry->getPackageName(); + + $url = str_replace( + ['%package%', '%version%'], + [$packageName, $entry->version], + self::PACKAGE_METADATA_PATTERN + ); + $packages[$packageName] = [ + $this->httpClient->request('GET', $url), + $dependencies, + ]; + } + + $errors = []; + $problems = []; + foreach ($packages as $packageName => [$response, $dependencies]) { + if (200 !== $response->getStatusCode()) { + $errors[] = [$packageName, $response]; + continue; + } + + $data = json_decode($response->getContent(), true); + // dependencies seem to be found in both places + $packageDependencies = array_merge( + $data['dependencies'] ?? [], + $data['peerDependencies'] ?? [] + ); + + foreach ($dependencies as $dependencyName) { + // dependency is not in the import map + if (!$entries->has($dependencyName)) { + $dependencyVersionConstraint = $packageDependencies[$dependencyName] ?? 'unknown'; + $problems[] = new PackageVersionProblem($packageName, $dependencyName, $dependencyVersionConstraint, null); + + continue; + } + + $dependencyPackageName = $entries->get($dependencyName)->getPackageName(); + $dependencyVersionConstraint = $packageDependencies[$dependencyPackageName] ?? null; + + if (null === $dependencyVersionConstraint) { + $problems[] = new PackageVersionProblem($packageName, $dependencyPackageName, $dependencyVersionConstraint, $entries->get($dependencyName)->version); + + continue; + } + + if (!$this->isVersionSatisfied($dependencyVersionConstraint, $entries->get($dependencyName)->version)) { + $problems[] = new PackageVersionProblem($packageName, $dependencyPackageName, $dependencyVersionConstraint, $entries->get($dependencyName)->version); + } + } + } + + try { + ($errors[0][1] ?? null)?->getHeaders(); + } catch (HttpExceptionInterface $e) { + $response = $e->getResponse(); + $packageNames = implode('", "', array_column($errors, 0)); + + throw new RuntimeException(sprintf('Error %d finding metadata for package "%s". Response: ', $response->getStatusCode(), $packageNames).$response->getContent(false), 0, $e); + } + + return $problems; + } + + /** + * Converts npm-specific version constraints to composer-style. + * + * @internal + */ + public static function convertNpmConstraint(string $versionConstraint): ?string + { + // special npm constraint that don't translate to composer + if (\in_array($versionConstraint, ['latest', 'next']) + || preg_match('/^(git|http|file):/', $versionConstraint) + || str_contains($versionConstraint, '/') + ) { + // GitHub shorthand like user/repo + return null; + } + + // remove whitespace around hyphens + $versionConstraint = preg_replace('/\s?-\s?/', '-', $versionConstraint); + $segments = explode(' ', $versionConstraint); + $processedSegments = []; + + foreach ($segments as $segment) { + if (str_contains($segment, '-') && !preg_match('/-(alpha|beta|rc)\./', $segment)) { + // This is a range + [$start, $end] = explode('-', $segment); + $processedSegments[] = '>='.self::cleanVersionSegment(trim($start)).' <='.self::cleanVersionSegment(trim($end)); + } elseif (preg_match('/^~(\d+\.\d+)$/', $segment, $matches)) { + // Handle the tilde when only major.minor specified + $baseVersion = $matches[1]; + $processedSegments[] = '>='.$baseVersion.'.0'; + $processedSegments[] = '<'.$baseVersion[0].'.'.($baseVersion[2] + 1).'.0'; + } else { + $processedSegments[] = self::cleanVersionSegment($segment); + } + } + + return implode(' ', $processedSegments); + } + + private static function cleanVersionSegment(string $segment): string + { + return str_replace(['v', '.x'], ['', '.*'], $segment); + } + + private function isVersionSatisfied(string $versionConstraint, ?string $version): bool + { + if (!$version) { + return false; + } + + try { + $versionConstraint = self::convertNpmConstraint($versionConstraint); + + // if version isn't parseable/convertible, assume it's not satisfied + if (null === $versionConstraint) { + return false; + } + + return Semver::satisfies($version, $versionConstraint); + } catch (\UnexpectedValueException $e) { + return false; + } + } +} diff --git a/src/Symfony/Component/AssetMapper/ImportMap/PackageVersionProblem.php b/src/Symfony/Component/AssetMapper/ImportMap/PackageVersionProblem.php new file mode 100644 index 0000000000000..26c17f4cddeb5 --- /dev/null +++ b/src/Symfony/Component/AssetMapper/ImportMap/PackageVersionProblem.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\AssetMapper\ImportMap; + +final class PackageVersionProblem +{ + public function __construct( + public readonly string $packageName, + public readonly string $dependencyPackageName, + public readonly ?string $requiredVersionConstraint, + public readonly ?string $installedVersion + ) { + } +} diff --git a/src/Symfony/Component/AssetMapper/ImportMap/RemotePackageDownloader.php b/src/Symfony/Component/AssetMapper/ImportMap/RemotePackageDownloader.php index 577abfd5e7236..782a8a9133e13 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/RemotePackageDownloader.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/RemotePackageDownloader.php @@ -71,9 +71,10 @@ public function downloadPackages(callable $progressCallback = null): array throw new \LogicException(sprintf('The package "%s" was not downloaded.', $package)); } - $this->remotePackageStorage->save($entry, $contents[$package]); + $this->remotePackageStorage->save($entry, $contents[$package]['content']); $newInstalled[$package] = [ 'version' => $entry->version, + 'dependencies' => $contents[$package]['dependencies'] ?? [], ]; $downloadedPackages[] = $package; @@ -89,13 +90,26 @@ public function downloadPackages(callable $progressCallback = null): array return $downloadedPackages; } + /** + * @return string[] + */ + public function getDependencies(string $importName): array + { + $installed = $this->loadInstalled(); + if (!isset($installed[$importName])) { + throw new \InvalidArgumentException(sprintf('The "%s" vendor asset is missing. Run "php bin/console importmap:install".', $importName)); + } + + return $installed[$importName]['dependencies']; + } + public function getVendorDir(): string { return $this->remotePackageStorage->getStorageDir(); } /** - * @return array + * @return array}> */ private function loadInstalled(): array { @@ -110,6 +124,10 @@ private function loadInstalled(): array if (!isset($data['version'])) { throw new \InvalidArgumentException(sprintf('The package "%s" is missing its version.', $package)); } + + if (!isset($data['dependencies'])) { + throw new \LogicException(sprintf('The package "%s" is missing its dependencies.', $package)); + } } return $this->installed = $installed; diff --git a/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php b/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php index 9b9f395a3a0aa..38f4e0f5e88f9 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php @@ -152,7 +152,7 @@ public function resolvePackages(array $packagesToRequire): array /** * @param ImportMapEntry[] $importMapEntries * - * @return array + * @return array */ public function downloadPackages(array $importMapEntries, callable $progressCallback = null): array { @@ -180,7 +180,12 @@ public function downloadPackages(array $importMapEntries, callable $progressCall if ($progressCallback) { $progressCallback($package, 'started', $response, \count($responses)); } - $contents[$package] = $this->makeImportsBare($response->getContent()); + + $dependencies = []; + $contents[$package] = [ + 'content' => $this->makeImportsBare($response->getContent(), $dependencies), + 'dependencies' => $dependencies, + ]; if ($progressCallback) { $progressCallback($package, 'finished', $response, \count($responses)); } @@ -226,9 +231,14 @@ private function fetchPackageRequirementsFromImports(string $content): array * * Replaces those with normal import "package/name" statements. */ - private function makeImportsBare(string $content): string + private function makeImportsBare(string $content, array &$dependencies): string { - $content = preg_replace_callback(self::IMPORT_REGEX, fn ($m) => sprintf('from"%s"', $m[1]), $content); + $content = preg_replace_callback(self::IMPORT_REGEX, function ($matches) use (&$dependencies) { + $packageName = $matches[1]; + $dependencies[] = $packageName; + + return sprintf('from"%s"', $packageName); + }, $content); // source maps are not also downloaded - so remove the sourceMappingURL $content = preg_replace('{//# sourceMappingURL=.*$}m', '', $content); diff --git a/src/Symfony/Component/AssetMapper/ImportMap/Resolver/PackageResolverInterface.php b/src/Symfony/Component/AssetMapper/ImportMap/Resolver/PackageResolverInterface.php index a569b06039d6a..41e3aa7222531 100644 --- a/src/Symfony/Component/AssetMapper/ImportMap/Resolver/PackageResolverInterface.php +++ b/src/Symfony/Component/AssetMapper/ImportMap/Resolver/PackageResolverInterface.php @@ -33,9 +33,11 @@ public function resolvePackages(array $packagesToRequire): array; * * The returned array should be a map using the same keys as $importMapEntries. * + * The dependencies are an array of module names that are imported by the package. + * * @param array $importMapEntries * - * @return array + * @return array */ public function downloadPackages(array $importMapEntries, callable $progressCallback = null): array; } diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapVersionCheckerTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapVersionCheckerTest.php new file mode 100644 index 0000000000000..b0c895b79536d --- /dev/null +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapVersionCheckerTest.php @@ -0,0 +1,436 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\AssetMapper\Tests\ImportMap; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader; +use Symfony\Component\AssetMapper\ImportMap\ImportMapEntries; +use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry; +use Symfony\Component\AssetMapper\ImportMap\ImportMapType; +use Symfony\Component\AssetMapper\ImportMap\ImportMapVersionChecker; +use Symfony\Component\AssetMapper\ImportMap\PackageVersionProblem; +use Symfony\Component\AssetMapper\ImportMap\RemotePackageDownloader; +use Symfony\Component\HttpClient\MockHttpClient; +use Symfony\Component\HttpClient\Response\MockResponse; + +class ImportMapVersionCheckerTest extends TestCase +{ + /** + * @dataProvider getCheckVersionsTests + */ + public function testCheckVersions(array $importMapEntries, array $dependencies, array $expectedRequests, array $expectedProblems) + { + $configReader = $this->createMock(ImportMapConfigReader::class); + $configReader->expects($this->once()) + ->method('getEntries') + ->willReturn(new ImportMapEntries($importMapEntries)); + + $remoteDownloader = $this->createMock(RemotePackageDownloader::class); + $remoteDownloader->expects($this->exactly(\count($importMapEntries))) + ->method('getDependencies') + ->with($this->callback(function ($importName) use ($importMapEntries) { + foreach ($importMapEntries as $entry) { + if ($entry->importName === $importName) { + return true; + } + } + + return false; + })) + ->willReturnCallback(function ($importName) use ($dependencies) { + if (!isset($dependencies[$importName])) { + throw new \InvalidArgumentException(sprintf('Missing dependencies in test for "%s"', $importName)); + } + + return $dependencies[$importName]; + }); + + $responses = []; + foreach ($expectedRequests as $expectedRequest) { + $responses[] = function ($method, $url) use ($expectedRequest) { + $this->assertStringEndsWith($expectedRequest['url'], $url); + + return new MockResponse(json_encode($expectedRequest['response'])); + }; + } + $httpClient = new MockHttpClient($responses); + + $versionChecker = new ImportMapVersionChecker($configReader, $remoteDownloader, $httpClient); + $problems = $versionChecker->checkVersions(); + $this->assertEquals($expectedProblems, $problems); + $this->assertSame(\count($expectedRequests), $httpClient->getRequestsCount()); + } + + public static function getCheckVersionsTests() + { + yield 'no dependencies' => [ + [ + self::createRemoteEntry('foo', '1.0.0'), + ], + [ + 'foo' => [], + ], + [], + [], + ]; + + yield 'single with dependency but no problem' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => '1.2.7 || 1.2.9- v2.0.0'], + ], + ], + ], + [], + ]; + + yield 'single with dependency with problem' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => '^2.0.0'], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', '^2.0.0', '1.5.0'), + ], + ]; + + yield 'single with dependency & different package specifier with problem' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0', packageModuleSpecifier: 'foo_package'), + self::createRemoteEntry('bar', version: '1.5.0', packageModuleSpecifier: 'bar_package'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo_package/1.0.0', + 'response' => [ + 'dependencies' => ['bar_package' => '^2.0.0'], + ], + ], + ], + [ + new PackageVersionProblem('foo_package', 'bar_package', '^2.0.0', '1.5.0'), + ], + ]; + + yield 'single with missing dependency' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + ], + [ + 'foo' => ['bar'], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => '^2.0.0'], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', '^2.0.0', null), + ], + ]; + + yield 'multiple package and problems' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + self::createRemoteEntry('baz', version: '2.0.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => ['baz'], + 'baz' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => '^2.0.0'], + ], + ], + [ + 'url' => '/bar/1.5.0', + 'response' => [ + 'dependencies' => ['baz' => '^1.0.0'], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', '^2.0.0', '1.5.0'), + new PackageVersionProblem('bar', 'baz', '^1.0.0', '2.0.0'), + ], + ]; + + yield 'single with problem on peerDependency' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'peerDependencies' => ['bar' => '^2.0.0'], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', '^2.0.0', '1.5.0'), + ], + ]; + + yield 'single that imports something that is not required by the package' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => [], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', null, '1.5.0'), + ], + ]; + + yield 'single with npm-style constraint' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => '1.0.0 - v2.0.0'], + ], + ], + ], + [], + ]; + + yield 'single with invalid constraint shows as problem' => [ + [ + self::createRemoteEntry('foo', version: '1.0.0'), + self::createRemoteEntry('bar', version: '1.5.0'), + ], + [ + 'foo' => ['bar'], + 'bar' => [], + ], + [ + [ + 'url' => '/foo/1.0.0', + 'response' => [ + 'dependencies' => ['bar' => 'some/repo'], + ], + ], + ], + [ + new PackageVersionProblem('foo', 'bar', 'some/repo', '1.5.0'), + ], + ]; + } + + /** + * @dataProvider getNpmSpecificVersionConstraints + */ + public function testNpmSpecificConstraints(string $npmConstraint, ?string $expectedComposerConstraint) + { + $this->assertSame($expectedComposerConstraint, ImportMapVersionChecker::convertNpmConstraint($npmConstraint)); + } + + public static function getNpmSpecificVersionConstraints() + { + // Simple cases + yield 'simple no change' => [ + '1.2.*', + '1.2.*', + ]; + + yield 'logical or with no change' => [ + '5.4.*|6.0.*', + '5.4.*|6.0.*', + ]; + + yield 'other or syntax, spaces, no change' => [ + '>1.2.7 || <1.0.0', + '>1.2.7 || <1.0.0', + ]; + + yield 'using v prefix' => [ + 'v1.2.*', + '1.2.*', + ]; + + // Hyphen Ranges + yield 'hyphen range simple' => [ + '1.0.0 - 2.0.0', + '>=1.0.0 <=2.0.0', + ]; + + yield 'hyphen range with v prefix' => [ + 'v1.0.0 - 2.0.0', + '>=1.0.0 <=2.0.0', + ]; + + yield 'hyphen range without patch' => [ + '1.0 - 2.0', + '>=1.0 <=2.0', + ]; + + yield 'hyphen range with no spaces' => [ + '1.0-v2.0', + '>=1.0 <=2.0', + ]; + + // .x Wildcards + yield '.x wildcard' => [ + '5.4.x', + '5.4.*', + ]; + + yield '.x wildcard without minor' => [ + '5.x', + '5.*', + ]; + + // Multiple Constraints with Spaces + yield 'multiple constraints' => [ + '>1.2.7 <=1.3.0', + '>1.2.7 <=1.3.0', + ]; + + yield 'multiple constraints with v' => [ + '>v1.2.7 <=v1.3.0', + '>1.2.7 <=1.3.0', + ]; + + yield 'mixed constraints with wildcard' => [ + '>=5.x <6.0.0', + '>=5.* <6.0.0', + ]; + + // Pre-release Versions + yield 'pre-release version' => [ + '1.2.3-beta.0', + '1.2.3-beta.0', + ]; + + yield 'pre-release with v prefix' => [ + 'v1.2.3-alpha.1', + '1.2.3-alpha.1', + ]; + + // Constraints that don't translate to Composer + yield 'latest tag' => [ + 'latest', + null, + ]; + + yield 'next tag' => [ + 'next', + null, + ]; + + yield 'local path' => [ + 'file:../my-lib', + null, + ]; + + yield 'git repository' => [ + 'git://github.com/user/project.git#commit-ish', + null, + ]; + + yield 'github shorthand' => [ + 'user/repo#semver:^1.0.0', + null, + ]; + + yield 'url' => [ + 'https://example.com/module.tgz', + null, + ]; + + yield 'multiple constraints with space and or operator' => [ + '1.2.7 || 1.2.9- v2.0.0', + '1.2.7 || >=1.2.9 <=2.0.0', + ]; + + yield 'tilde constraint with patch version no change' => [ + '~1.2.3', + '~1.2.3', + ]; + + yield 'tilde constraint with minor version changes' => [ + '~1.2', + '>=1.2.0 <1.3.0', + ]; + + yield 'tilde constraint with major version no change' => [ + '~1', + '~1', + ]; + } + + private static function createRemoteEntry(string $importName, string $version, string $packageModuleSpecifier = null): ImportMapEntry + { + $packageModuleSpecifier = $packageModuleSpecifier ?? $importName; + + return ImportMapEntry::createRemote($importName, ImportMapType::JS, '/path/to/'.$importName, $version, $packageModuleSpecifier, false); + } +} diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/RemotePackageDownloaderTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/RemotePackageDownloaderTest.php index 26c5ee8769d10..89975b6ea61d2 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/RemotePackageDownloaderTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/RemotePackageDownloaderTest.php @@ -62,7 +62,12 @@ public function testDownloadPackagesDownloadsEverythingWithNoInstalled() ['foo' => $entry1, 'bar.js/file' => $entry2, 'baz' => $entry3, 'different_specifier' => $entry4], $progressCallback ) - ->willReturn(['foo' => 'foo content', 'bar.js/file' => 'bar content', 'baz' => 'baz content', 'different_specifier' => 'different content']); + ->willReturn([ + 'foo' => ['content' => 'foo content', 'dependencies' => []], + 'bar.js/file' => ['content' => 'bar content', 'dependencies' => []], + 'baz' => ['content' => 'baz content', 'dependencies' => ['foo']], + 'different_specifier' => ['content' => 'different content', 'dependencies' => []], + ]); $downloader = new RemotePackageDownloader( $remotePackageStorage, @@ -82,10 +87,10 @@ public function testDownloadPackagesDownloadsEverythingWithNoInstalled() $installed = require self::$writableRoot.'/assets/vendor/installed.php'; $this->assertEquals( [ - 'foo' => ['version' => '1.0.0'], - 'bar.js/file' => ['version' => '1.0.0'], - 'baz' => ['version' => '1.0.0'], - 'different_specifier' => ['version' => '1.0.0'], + 'foo' => ['version' => '1.0.0', 'dependencies' => []], + 'bar.js/file' => ['version' => '1.0.0', 'dependencies' => []], + 'baz' => ['version' => '1.0.0', 'dependencies' => ['foo']], + 'different_specifier' => ['version' => '1.0.0', 'dependencies' => []], ], $installed ); @@ -95,9 +100,9 @@ public function testPackagesWithCorrectInstalledVersionSkipped() { $this->filesystem->mkdir(self::$writableRoot.'/assets/vendor'); $installed = [ - 'foo' => ['version' => '1.0.0'], - 'bar.js/file' => ['version' => '1.0.0'], - 'baz' => ['version' => '1.0.0'], + 'foo' => ['version' => '1.0.0', 'dependencies' => []], + 'bar.js/file' => ['version' => '1.0.0', 'dependencies' => []], + 'baz' => ['version' => '1.0.0', 'dependencies' => []], ]; file_put_contents( self::$writableRoot.'/assets/vendor/installed.php', @@ -125,7 +130,10 @@ public function testPackagesWithCorrectInstalledVersionSkipped() $packageResolver->expects($this->once()) ->method('downloadPackages') - ->willReturn(['bar.js/file' => 'new bar content', 'baz' => 'new baz content']); + ->willReturn([ + 'bar.js/file' => ['content' => 'new bar content', 'dependencies' => []], + 'baz' => ['content' => 'new baz content', 'dependencies' => []], + ]); $downloader = new RemotePackageDownloader( new RemotePackageStorage(self::$writableRoot.'/assets/vendor'), @@ -144,9 +152,9 @@ public function testPackagesWithCorrectInstalledVersionSkipped() $installed = require self::$writableRoot.'/assets/vendor/installed.php'; $this->assertEquals( [ - 'foo' => ['version' => '1.0.0'], - 'bar.js/file' => ['version' => '1.0.0'], - 'baz' => ['version' => '1.1.0'], + 'foo' => ['version' => '1.0.0', 'dependencies' => []], + 'bar.js/file' => ['version' => '1.0.0', 'dependencies' => []], + 'baz' => ['version' => '1.1.0', 'dependencies' => []], ], $installed ); diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php index fb29df4ad53e5..75035ec56c8dc 100644 --- a/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php +++ b/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php @@ -268,7 +268,7 @@ public static function provideResolvePackagesTests(): iterable /** * @dataProvider provideDownloadPackagesTests */ - public function testDownloadPackages(array $importMapEntries, array $expectedRequests, array $expectedContents) + public function testDownloadPackages(array $importMapEntries, array $expectedRequests, array $expectedReturn, array $expectedDependencies = []) { $responses = []; foreach ($expectedRequests as $expectedRequest) { @@ -283,10 +283,14 @@ public function testDownloadPackages(array $importMapEntries, array $expectedReq $httpClient = new MockHttpClient($responses); $provider = new JsDelivrEsmResolver($httpClient); - $actualContents = $provider->downloadPackages($importMapEntries); - $this->assertCount(\count($expectedContents), $actualContents); - $actualContents = array_map('trim', $actualContents); - $this->assertSame($expectedContents, $actualContents); + $actualReturn = $provider->downloadPackages($importMapEntries); + + foreach ($actualReturn as $key => $data) { + $actualReturn[$key]['content'] = trim($data['content']); + } + $this->assertCount(\count($expectedReturn), $actualReturn); + + $this->assertSame($expectedReturn, $actualReturn); $this->assertSame(\count($expectedRequests), $httpClient->getRequestsCount()); } @@ -301,7 +305,7 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'lodash contents', + 'lodash' => ['content' => 'lodash contents', 'dependencies' => []], ], ]; @@ -314,7 +318,7 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'lodash contents', + 'lodash' => ['content' => 'lodash contents', 'dependencies' => []], ], ]; @@ -327,7 +331,7 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'chart.js contents', + 'lodash' => ['content' => 'chart.js contents', 'dependencies' => []], ], ]; @@ -340,7 +344,7 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'bootstrap.css contents', + 'lodash' => ['content' => 'bootstrap.css contents', 'dependencies' => []], ], ]; @@ -365,9 +369,9 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'lodash contents', - 'chart.js/auto' => 'chart.js contents', - 'bootstrap/dist/bootstrap.css' => 'bootstrap.css contents', + 'lodash' => ['content' => 'lodash contents', 'dependencies' => []], + 'chart.js/auto' => ['content' => 'chart.js contents', 'dependencies' => []], + 'bootstrap/dist/bootstrap.css' => ['content' => 'bootstrap.css contents', 'dependencies' => []], ], ]; @@ -382,11 +386,14 @@ public static function provideDownloadPackagesTests() ], ], [ - '@chart.js/auto' => 'import{Color as t}from"@kurkle/color";function e(){}const i=(()=', + '@chart.js/auto' => [ + 'content' => 'import{Color as t}from"@kurkle/color";function e(){}const i=(()=', + 'dependencies' => ['@kurkle/color'], + ], ], ]; - yield 'js importmap is removed' => [ + yield 'js sourcemap is removed' => [ [ '@chart.js/auto' => self::createRemoteEntry('chart.js/auto', version: '1.2.3'), ], @@ -398,7 +405,10 @@ public static function provideDownloadPackagesTests() ], ], [ - '@chart.js/auto' => 'as Ticks,ta as TimeScale,ia as TimeSeriesScale,oo as Title,wo as Tooltip,Ci as _adapters,us as _detectPlatform,Ye as animator,Si as controllers,tn as default,St as defaults,Pn as elements,qi as layouts,ko as plugins,na as registerables,Ps as registry,sa as scales};', + '@chart.js/auto' => [ + 'content' => 'as Ticks,ta as TimeScale,ia as TimeSeriesScale,oo as Title,wo as Tooltip,Ci as _adapters,us as _detectPlatform,Ye as animator,Si as controllers,tn as default,St as defaults,Pn as elements,qi as layouts,ko as plugins,na as registerables,Ps as registry,sa as scales};', + 'dependencies' => [], + ], ], ]; @@ -412,7 +422,10 @@ public static function provideDownloadPackagesTests() ], ], [ - 'lodash' => 'print-table-row{display:table-row!important}.d-print-table-cell{display:table-cell!important}.d-print-flex{display:flex!important}.d-print-inline-flex{display:inline-flex!important}.d-print-none{display:none!important}}', + 'lodash' => [ + 'content' => 'print-table-row{display:table-row!important}.d-print-table-cell{display:table-cell!important}.d-print-flex{display:flex!important}.d-print-inline-flex{display:inline-flex!important}.d-print-none{display:none!important}}', + 'dependencies' => [], + ], ], ]; } diff --git a/src/Symfony/Component/AssetMapper/Tests/fixtures/assets/vendor/installed.php b/src/Symfony/Component/AssetMapper/Tests/fixtures/assets/vendor/installed.php index 1d86382dcfc3f..b11266b993c57 100644 --- a/src/Symfony/Component/AssetMapper/Tests/fixtures/assets/vendor/installed.php +++ b/src/Symfony/Component/AssetMapper/Tests/fixtures/assets/vendor/installed.php @@ -2,9 +2,11 @@ '@hotwired/stimulus' => array ( 'version' => '3.2.1', + 'dependencies' => array(), ), 'lodash' => array ( 'version' => '4.17.21', + 'dependencies' => array(), ), ); diff --git a/src/Symfony/Component/AssetMapper/composer.json b/src/Symfony/Component/AssetMapper/composer.json index 973a0ff747407..d0c6f733cda9e 100644 --- a/src/Symfony/Component/AssetMapper/composer.json +++ b/src/Symfony/Component/AssetMapper/composer.json @@ -17,6 +17,7 @@ ], "require": { "php": ">=8.1", + "composer/semver": "^3.0", "symfony/deprecation-contracts": "^2.5|^3", "symfony/filesystem": "^5.4|^6.0|^7.0", "symfony/http-client": "^6.3|^7.0"