[AssetMapper] Using ?specifier=* does not match unstable packages on jsdelivr

weaverryan · weaverryan · commit 370f6dc14581 · 2023-10-19T14:11:40.000-04:00
diff --git a/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php b/src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php
@@ -21,7 +21,7 @@
 
 final class JsDelivrEsmResolver implements PackageResolverInterface
 {
-    public const URL_PATTERN_VERSION = 'https://data.jsdelivr.com/v1/packages/npm/%s/resolved?specifier=%s';
+    public const URL_PATTERN_VERSION = 'https://data.jsdelivr.com/v1/packages/npm/%s/resolved';
     public const URL_PATTERN_DIST_CSS = 'https://cdn.jsdelivr.net/npm/%s@%s%s';
     public const URL_PATTERN_DIST = self::URL_PATTERN_DIST_CSS.'/+esm';
     public const URL_PATTERN_ENTRYPOINT = 'https://data.jsdelivr.com/v1/packages/npm/%s@%s/entrypoints';
@@ -32,9 +32,6 @@ final class JsDelivrEsmResolver implements PackageResolverInterface
 
     public function __construct(
         HttpClientInterface $httpClient = null,
-        private readonly string $versionUrlPattern = self::URL_PATTERN_VERSION,
-        private readonly string $distUrlPattern = self::URL_PATTERN_DIST,
-        private readonly string $distUrlCssPattern = self::URL_PATTERN_DIST_CSS
     ) {
         $this->httpClient = $httpClient ?? HttpClient::create();
     }
@@ -49,7 +46,6 @@ public function resolvePackages(array $packagesToRequire): array
         $requiredPackages = [];
         foreach ($packagesToRequire as $options) {
             $packageSpecifier = trim($options->packageModuleSpecifier, '/');
-            $constraint = $options->versionConstraint ?? '*';
 
             // avoid resolving the same package twice
             if (isset($resolvedPackages[$packageSpecifier])) {
@@ -58,7 +54,11 @@ public function resolvePackages(array $packagesToRequire): array
 
             [$packageName, $filePath] = ImportMapEntry::splitPackageNameAndFilePath($packageSpecifier);
 
-            $response = $this->httpClient->request('GET', sprintf($this->versionUrlPattern, $packageName, urlencode($constraint)));
+            $versionUrl = sprintf(self::URL_PATTERN_VERSION, $packageName);
+            if (null !== $options->versionConstraint) {
+                $versionUrl .= '?specifier='.urlencode($options->versionConstraint);
+            }
+            $response = $this->httpClient->request('GET', $versionUrl);
             $requiredPackages[] = [$options, $response, $packageName, $filePath, /* resolved version */ null];
         }
 
@@ -72,7 +72,11 @@ public function resolvePackages(array $packagesToRequire): array
             }
 
             $version = $response->toArray()['version'];
-            $pattern = str_ends_with($filePath, '.css') ? $this->distUrlCssPattern : $this->distUrlPattern;
+            if (null === $version) {
+                throw new RuntimeException(sprintf('Unable to find the latest version for package "%s" - try specifying the version manually.', $packageName));
+            }
+
+            $pattern = str_ends_with($filePath, '.css') ? self::URL_PATTERN_DIST_CSS : self::URL_PATTERN_DIST;
             $requiredPackages[$i][1] = $this->httpClient->request('GET', sprintf($pattern, $packageName, $version, $filePath));
             $requiredPackages[$i][4] = $version;
 
@@ -163,7 +167,7 @@ public function downloadPackages(array $importMapEntries, callable $progressCall
                 throw new \InvalidArgumentException(sprintf('The entry "%s" is not a remote package.', $entry->importName));
             }
 
-            $pattern = ImportMapType::CSS === $entry->type ? $this->distUrlCssPattern : $this->distUrlPattern;
+            $pattern = ImportMapType::CSS === $entry->type ? self::URL_PATTERN_DIST_CSS : self::URL_PATTERN_DIST;
             $url = sprintf($pattern, $entry->getPackageName(), $entry->version, $entry->getPackagePathString());
 
             $responses[$package] = $this->httpClient->request('GET', $url);
diff --git a/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php b/src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php
@@ -61,7 +61,7 @@ public static function provideResolvePackagesTests(): iterable
             'packages' => [new PackageRequireOptions('lodash')],
             'expectedRequests' => [
                 [
-                    'url' => '/v1/packages/npm/lodash/resolved?specifier=%2A',
+                    'url' => '/v1/packages/npm/lodash/resolved',
                     'response' => ['body' => ['version' => '1.2.3']],
                 ],
                 [
@@ -196,7 +196,7 @@ public static function provideResolvePackagesTests(): iterable
             'packages' => [new PackageRequireOptions('bootstrap/dist/css/bootstrap.min.css')],
             'expectedRequests' => [
                 [
-                    'url' => '/v1/packages/npm/bootstrap/resolved?specifier=%2A',
+                    'url' => '/v1/packages/npm/bootstrap/resolved',
                     'response' => ['body' => ['version' => '3.3.0']],
                 ],
                 [

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ public static function provideResolvePackagesTests(): iterable`
`61`	`61`	`'packages' => [new PackageRequireOptions('lodash')],`
`62`	`62`	`'expectedRequests' => [`
`63`	`63`	`[`
`64`		`- 'url' => '/v1/packages/npm/lodash/resolved?specifier=%2A',`
	`64`	`+ 'url' => '/v1/packages/npm/lodash/resolved',`
`65`	`65`	`'response' => ['body' => ['version' => '1.2.3']],`
`66`	`66`	`],`
`67`	`67`	`[`
`@@ -196,7 +196,7 @@ public static function provideResolvePackagesTests(): iterable`
`196`	`196`	`'packages' => [new PackageRequireOptions('bootstrap/dist/css/bootstrap.min.css')],`
`197`	`197`	`'expectedRequests' => [`
`198`	`198`	`[`
`199`		`- 'url' => '/v1/packages/npm/bootstrap/resolved?specifier=%2A',`
	`199`	`+ 'url' => '/v1/packages/npm/bootstrap/resolved',`
`200`	`200`	`'response' => ['body' => ['version' => '3.3.0']],`
`201`	`201`	`],`
`202`	`202`	`[`