Merge branch '1.x' into 2.x

* 1.x: AssetMapper: upgrade packages when needed
symfony · Oct 22, 2023 · 84d8824 · 84d8824
2 parents 84de0f9 + 884a881
commit 84d8824
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 38 deletions.
diff --git a/src/Flex.php b/src/Flex.php
@@ -502,7 +502,7 @@ private function synchronizePackageJson(string $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);
+        $synchronizer = new PackageJsonSynchronizer($rootDir, $vendorDir, $executor, $this->io);
 
         if ($synchronizer->shouldSynchronize()) {
             $lockData = $this->composer->getLocker()->getLockData();

diff --git a/src/PackageJsonSynchronizer.php b/src/PackageJsonSynchronizer.php
@@ -11,8 +11,10 @@
 
 namespace Symfony\Flex;
 
+use Composer\IO\IOInterface;
 use Composer\Json\JsonFile;
 use Composer\Json\JsonManipulator;
+use Composer\Semver\Semver;
 use Composer\Semver\VersionParser;
 use Seld\JsonLint\ParsingException;
 
@@ -25,13 +27,15 @@ class PackageJsonSynchronizer
     private $rootDir;
     private $vendorDir;
     private $scriptExecutor;
+    private $io;
     private $versionParser;
 
-    public function __construct(string $rootDir, string $vendorDir, ScriptExecutor $scriptExecutor)
+    public function __construct(string $rootDir, string $vendorDir, ScriptExecutor $scriptExecutor, IOInterface $io)
     {
         $this->rootDir = $rootDir;
         $this->vendorDir = $vendorDir;
         $this->scriptExecutor = $scriptExecutor;
+        $this->io = $io;
         $this->versionParser = new VersionParser();
     }
 
@@ -147,11 +151,9 @@ private function resolveImportMapPackages($phpPackage): array
         foreach ($packageJson->read()['symfony']['importmap'] ?? [] as $importMapName => $constraintConfig) {
             if (\is_array($constraintConfig)) {
                 $constraint = $constraintConfig['version'] ?? [];
-                $preload = $constraintConfig['preload'] ?? false;
                 $package = $constraintConfig['package'] ?? $importMapName;
             } else {
                 $constraint = $constraintConfig;
-                $preload = false;
                 $package = $importMapName;
             }
 
@@ -161,7 +163,6 @@ private function resolveImportMapPackages($phpPackage): array
 
                 $dependencies[$importMapName] = [
                     'path' => $path,
-                    'preload' => $preload,
                 ];
 
                 continue;
@@ -170,7 +171,6 @@ private function resolveImportMapPackages($phpPackage): array
             $dependencies[$importMapName] = [
                 'version' => $constraint,
                 'package' => $package,
-                'preload' => $preload,
             ];
         }
 
@@ -239,7 +239,7 @@ private function shouldUpdateConstraint(string $existingConstraint, string $cons
     }
 
     /**
-     * @param array<string, array{path?: string, preload: bool, package?: string, version?: string}> $importMapEntries
+     * @param array<string, array{path?: string, package?: string, version?: string}> $importMapEntries
      */
     private function updateImportMap(array $importMapEntries): void
     {
@@ -251,14 +251,24 @@ private function updateImportMap(array $importMapEntries): void
 
         foreach ($importMapEntries as $name => $importMapEntry) {
             if (isset($importMapData[$name])) {
-                continue;
+                if (!isset($importMapData[$name]['version'])) {
+                    // AssetMapper 6.3
+                    continue;
+                }
+
+                $version = $importMapData[$name]['version'];
+                $versionConstraint = $importMapEntry['version'] ?? null;
+
+                // if the version constraint is satisfied, skip - else, update the package
+                if (Semver::satisfies($version, $versionConstraint)) {
+                    continue;
+                }
+
+                $this->io->writeError(sprintf('Updating package <comment>%s</> from <info>%s</> to <info>%s</>.', $name, $version, $versionConstraint));
             }
 
             if (isset($importMapEntry['path'])) {
                 $arguments = [$name, '--path='.$importMapEntry['path']];
-                if ($importMapEntry['preload']) {
-                    $arguments[] = '--preload';
-                }
                 $this->scriptExecutor->execute(
                     'symfony-cmd',
                     'importmap:require',
@@ -274,9 +284,6 @@ private function updateImportMap(array $importMapEntries): void
                     $packageName .= '='.$name;
                 }
                 $arguments = [$packageName];
-                if ($importMapEntry['preload']) {
-                    $arguments[] = '--preload';
-                }
                 $this->scriptExecutor->execute(
                     'symfony-cmd',
                     'importmap:require',

diff --git a/tests/Fixtures/packageJson/elevated_dependencies_package.json b/tests/Fixtures/packageJson/elevated_dependencies_package.json
@@ -1,8 +1,8 @@
 {
    "name": "symfony/fixture",
    "dependencies": {
-      "@hotcookies": "^1.1|^2",
-      "@hotdogs": "^2",
+      "@hotcookies/bar": "^1.1|^2",
+      "@hotdogs/bun": "^2",
       "@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets"
    },
    "devDependencies": {

diff --git a/tests/Fixtures/packageJson/stricter_constraints_package.json b/tests/Fixtures/packageJson/stricter_constraints_package.json
@@ -1,8 +1,8 @@
 {
    "name": "symfony/fixture",
    "devDependencies": {
-      "@hotcookies": "^2",
-      "@hotdogs": "^1.9",
+      "@hotcookies/bar": "^2",
+      "@hotdogs/bun": "^1.9",
       "@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets"
    },
    "browserslist": [

diff --git a/tests/Fixtures/packageJson/vendor/symfony/existing-package/Resources/assets/package.json b/tests/Fixtures/packageJson/vendor/symfony/existing-package/Resources/assets/package.json
@@ -12,7 +12,7 @@
         }
     },
     "peerDependencies": {
-        "@hotcookies": "^1.1|^2",
-        "@hotdogs": "^2"
+        "@hotcookies/bar": "^1.1|^2",
+        "@hotdogs/bun": "^2"
     }
 }
diff --git a/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json b/tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json
@@ -11,14 +11,13 @@
         },
         "entrypoints": ["admin.js"],
         "importmap": {
-            "@hotcake": "^1.9.0",
+            "@hotcake/foo": "^1.9.0",
             "@symfony/new-package": {
-                "version": "path:%PACKAGE%/dist/loader.js",
-                "preload": true
+                "version": "path:%PACKAGE%/dist/loader.js"
             }
         }
     },
     "peerDependencies": {
-        "@hotcookies": "^1.1"
+        "@hotcookies/bar": "^1.1"
     }
 }
diff --git a/tests/Fixtures/packageJson/vendor/symfony/package-no-file-package/assets/package.json b/tests/Fixtures/packageJson/vendor/symfony/package-no-file-package/assets/package.json
@@ -3,6 +3,6 @@
         "needsPackageAsADependency": false
     },
     "peerDependencies": {
-        "@hotcookies": "^1.1"
+        "@hotcookies/bar": "^1.1"
     }
 }
diff --git a/tests/PackageJsonSynchronizerTest.php b/tests/PackageJsonSynchronizerTest.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Flex\Tests;
 
+use Composer\IO\IOInterface;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Flex\PackageJsonSynchronizer;
@@ -31,7 +32,8 @@ protected function setUp(): void
         $this->synchronizer = new PackageJsonSynchronizer(
             $this->tempDir,
             'vendor',
-            $this->scriptExecutor
+            $this->scriptExecutor,
+            $this->createMock(IOInterface::class)
         );
     }
 
@@ -99,8 +101,8 @@ public function testSynchronizeExistingPackage()
             [
                 'name' => 'symfony/fixture',
                 'devDependencies' => [
-                    '@hotcookies' => '^1.1|^2',
-                    '@hotdogs' => '^2',
+                    '@hotcookies/bar' => '^1.1|^2',
+                    '@hotdogs/bun' => '^2',
                     '@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
                     '@symfony/stimulus-bridge' => '^1.0.0',
                     'stimulus' => '^1.1.1',
@@ -151,7 +153,7 @@ public function testSynchronizeNewPackage()
             '{
    "name": "symfony/fixture",
    "devDependencies": {
-      "@hotdogs": "^2",
+      "@hotdogs/bun": "^2",
       "@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
       "@symfony/new-package": "file:vendor/symfony/new-package/assets",
       "@symfony/stimulus-bridge": "^1.0.0",
@@ -207,8 +209,8 @@ public function testArrayFormattingHasNotChanged()
             '{
    "name": "symfony/fixture",
    "devDependencies": {
-      "@hotcookies": "^1.1|^2",
-      "@hotdogs": "^2",
+      "@hotcookies/bar": "^1.1|^2",
+      "@hotdogs/bun": "^2",
       "@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
       "@symfony/stimulus-bridge": "^1.0.0",
       "stimulus": "^1.1.1"
@@ -237,8 +239,8 @@ public function testExistingElevatedPackage()
             [
                 'name' => 'symfony/fixture',
                 'dependencies' => [
-                    '@hotcookies' => '^1.1|^2',
-                    '@hotdogs' => '^2',
+                    '@hotcookies/bar' => '^1.1|^2',
+                    '@hotdogs/bun' => '^2',
                     '@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
                 ],
                 'devDependencies' => [
@@ -272,9 +274,9 @@ public function testStricterConstraintsAreKeptNonMatchingAreReplaced()
                 'name' => 'symfony/fixture',
                 'devDependencies' => [
                     // this satisfies the constraint, so it's kept
-                    '@hotcookies' => '^2',
+                    '@hotcookies/bar' => '^2',
                     // this was too low, so it's replaced
-                    '@hotdogs' => '^2',
+                    '@hotdogs/bun' => '^2',
                     '@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
                 ],
                 'browserslist' => [
@@ -303,7 +305,7 @@ public function testSynchronizePackageWithoutNeedingFilePackage()
             '{
    "name": "symfony/fixture",
    "devDependencies": {
-      "@hotdogs": "^2",
+      "@hotdogs/bun": "^2",
       "@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
       "@symfony/stimulus-bridge": "^1.0.0",
       "stimulus": "^1.1.1"
@@ -324,12 +326,13 @@ public function testSynchronizeAssetMapperNewPackage()
         $this->scriptExecutor->expects($this->exactly(2))
             ->method('execute')
             ->withConsecutive(
-                ['symfony-cmd', 'importmap:require', ['@hotcake@^1.9.0']],
-                ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath, '--preload']]
+                ['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']],
+                ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
             );
 
         $this->synchronizer->synchronize([
             [
+                // no "importmap" specific config, but still registered as a controller
                 'name' => 'symfony/existing-package',
                 'keywords' => ['symfony-ux'],
             ],
@@ -384,4 +387,55 @@ public function testSynchronizeAssetMapperNewPackage()
             json_decode(file_get_contents($this->tempDir.'/assets/controllers.json'), true)
         );
     }
+
+    public function testSynchronizeAssetMapperUpgradesPackageIfNeeded()
+    {
+        $importMap = [
+            '@hotcake/foo' => [
+                // constraint in package.json is ^1.9.0
+                'version' => '1.8.0',
+            ],
+        ];
+        file_put_contents($this->tempDir.'/importmap.php', sprintf('<?php return %s;', var_export($importMap, true)));
+
+        $fileModulePath = $this->tempDir.'/vendor/symfony/new-package/assets/dist/loader.js';
+        $this->scriptExecutor->expects($this->exactly(2))
+            ->method('execute')
+            ->withConsecutive(
+                ['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']],
+                ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
+            );
+
+        $this->synchronizer->synchronize([
+            [
+                'name' => 'symfony/new-package',
+                'keywords' => ['symfony-ux'],
+            ],
+        ]);
+    }
+
+    public function testSynchronizeAssetMapperSkipsUpgradeIfAlreadySatisfied()
+    {
+        $importMap = [
+            '@hotcake/foo' => [
+                // constraint in package.json is ^1.9.0
+                'version' => '1.9.1',
+            ],
+        ];
+        file_put_contents($this->tempDir.'/importmap.php', sprintf('<?php return %s;', var_export($importMap, true)));
+
+        $fileModulePath = $this->tempDir.'/vendor/symfony/new-package/assets/dist/loader.js';
+        $this->scriptExecutor->expects($this->once())
+            ->method('execute')
+            ->withConsecutive(
+                ['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
+            );
+
+        $this->synchronizer->synchronize([
+            [
+                'name' => 'symfony/new-package',
+                'keywords' => ['symfony-ux'],
+            ],
+        ]);
+    }
 }