Skip to content

Commit 884a881

Browse files
fabpotfabpot
committed
bug #996 AssetMapper: upgrade packages when needed (weaverryan)
This PR was squashed before being merged into the 1.x branch. Discussion ---------- AssetMapper: upgrade packages when needed Hi! The scenario: * User installs `symfony/ux-autocomplete`. Flex adds `tom-select` at version `2.2.3` to `importmap.php` * 3 months later, user upgrades `symfony/ux-autocomplete`. The new version now requires `tom-select` at `^2.5`. If we do nothing, the user's `tom-select` is out of date and the user won't even know about it. The `^2.5` constraint is defined in the `symfony/ux-autocomplete` [package.json file](https://github.com/symfony/ux/blob/97fe2cd62e70e329b85570e0b797833882930754/src/Autocomplete/assets/package.json#L23), but nothing enforces that or notifies the user. With this small change, during a composer update/require, we look at the dependencies of all UX packages and compare them against the version in `importmap.php`. If they do not match, the package is upgraded to a version that matches. <img width="995" alt="Screenshot 2023-10-22 at 1 45 41 PM" src="https://github.com/symfony/flex/assets/121003/286e58b0-d73d-4d7a-bc19-f748b18f489b"> We're not creating a fully-fledge package management by any means, but we don't need to. With a few notifications and things like this, we can keep the user's dependencies in sync with each other. Unrelated: don't forget about that really cool other PR #978 ;) Cheers! Commits ------- 5214615 AssetMapper: upgrade packages when needed
2 parents 333abcf + 5214615 commit 884a881

File tree

8 files changed

+98
-38
lines changed

8 files changed

+98
-38
lines changed

‎src/Flex.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ private function synchronizePackageJson(string $rootDir)
600600
$vendorDir = trim((new Filesystem())->makePathRelative($this->config->get('vendor-dir'), $rootDir), '/');
601601

602602
$executor = new ScriptExecutor($this->composer, $this->io, $this->options);
603-
$synchronizer = new PackageJsonSynchronizer($rootDir, $vendorDir, $executor);
603+
$synchronizer = new PackageJsonSynchronizer($rootDir, $vendorDir, $executor, $this->io);
604604

605605
if ($synchronizer->shouldSynchronize()) {
606606
$lockData = $this->composer->getLocker()->getLockData();

‎src/PackageJsonSynchronizer.php‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111

1212
namespace Symfony\Flex;
1313

14+
use Composer\IO\IOInterface;
1415
use Composer\Json\JsonFile;
1516
use Composer\Json\JsonManipulator;
17+
use Composer\Semver\Semver;
1618
use Composer\Semver\VersionParser;
1719
use Seld\JsonLint\ParsingException;
1820

@@ -25,13 +27,15 @@ class PackageJsonSynchronizer
2527
private $rootDir;
2628
private $vendorDir;
2729
private $scriptExecutor;
30+
private $io;
2831
private $versionParser;
2932

30-
public function __construct(string $rootDir, string $vendorDir, ScriptExecutor $scriptExecutor)
33+
public function __construct(string $rootDir, string $vendorDir, ScriptExecutor $scriptExecutor, IOInterface $io)
3134
{
3235
$this->rootDir = $rootDir;
3336
$this->vendorDir = $vendorDir;
3437
$this->scriptExecutor = $scriptExecutor;
38+
$this->io = $io;
3539
$this->versionParser = new VersionParser();
3640
}
3741

@@ -147,11 +151,9 @@ private function resolveImportMapPackages($phpPackage): array
147151
foreach ($packageJson->read()['symfony']['importmap'] ?? [] as $importMapName => $constraintConfig) {
148152
if (\is_array($constraintConfig)) {
149153
$constraint = $constraintConfig['version'] ?? [];
150-
$preload = $constraintConfig['preload'] ?? false;
151154
$package = $constraintConfig['package'] ?? $importMapName;
152155
} else {
153156
$constraint = $constraintConfig;
154-
$preload = false;
155157
$package = $importMapName;
156158
}
157159

@@ -161,7 +163,6 @@ private function resolveImportMapPackages($phpPackage): array
161163

162164
$dependencies[$importMapName] = [
163165
'path' => $path,
164-
'preload' => $preload,
165166
];
166167

167168
continue;
@@ -170,7 +171,6 @@ private function resolveImportMapPackages($phpPackage): array
170171
$dependencies[$importMapName] = [
171172
'version' => $constraint,
172173
'package' => $package,
173-
'preload' => $preload,
174174
];
175175
}
176176

@@ -239,7 +239,7 @@ private function shouldUpdateConstraint(string $existingConstraint, string $cons
239239
}
240240

241241
/**
242-
* @param array<string, array{path?: string, preload: bool, package?: string, version?: string}> $importMapEntries
242+
* @param array<string, array{path?: string, package?: string, version?: string}> $importMapEntries
243243
*/
244244
private function updateImportMap(array $importMapEntries): void
245245
{
@@ -251,14 +251,24 @@ private function updateImportMap(array $importMapEntries): void
251251

252252
foreach ($importMapEntries as $name => $importMapEntry) {
253253
if (isset($importMapData[$name])) {
254-
continue;
254+
if (!isset($importMapData[$name]['version'])) {
255+
// AssetMapper 6.3
256+
continue;
257+
}
258+
259+
$version = $importMapData[$name]['version'];
260+
$versionConstraint = $importMapEntry['version'] ?? null;
261+
262+
// if the version constraint is satisfied, skip - else, update the package
263+
if (Semver::satisfies($version, $versionConstraint)) {
264+
continue;
265+
}
266+
267+
$this->io->writeError(sprintf('Updating package <comment>%s</> from <info>%s</> to <info>%s</>.', $name, $version, $versionConstraint));
255268
}
256269

257270
if (isset($importMapEntry['path'])) {
258271
$arguments = [$name, '--path='.$importMapEntry['path']];
259-
if ($importMapEntry['preload']) {
260-
$arguments[] = '--preload';
261-
}
262272
$this->scriptExecutor->execute(
263273
'symfony-cmd',
264274
'importmap:require',
@@ -274,9 +284,6 @@ private function updateImportMap(array $importMapEntries): void
274284
$packageName .= '='.$name;
275285
}
276286
$arguments = [$packageName];
277-
if ($importMapEntry['preload']) {
278-
$arguments[] = '--preload';
279-
}
280287
$this->scriptExecutor->execute(
281288
'symfony-cmd',
282289
'importmap:require',

‎tests/Fixtures/packageJson/elevated_dependencies_package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
"name": "symfony/fixture",
33
"dependencies": {
4-
"@hotcookies": "^1.1|^2",
5-
"@hotdogs": "^2",
4+
"@hotcookies/bar": "^1.1|^2",
5+
"@hotdogs/bun": "^2",
66
"@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets"
77
},
88
"devDependencies": {

‎tests/Fixtures/packageJson/stricter_constraints_package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
"name": "symfony/fixture",
33
"devDependencies": {
4-
"@hotcookies": "^2",
5-
"@hotdogs": "^1.9",
4+
"@hotcookies/bar": "^2",
5+
"@hotdogs/bun": "^1.9",
66
"@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets"
77
},
88
"browserslist": [

‎tests/Fixtures/packageJson/vendor/symfony/existing-package/Resources/assets/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
}
1313
},
1414
"peerDependencies": {
15-
"@hotcookies": "^1.1|^2",
16-
"@hotdogs": "^2"
15+
"@hotcookies/bar": "^1.1|^2",
16+
"@hotdogs/bun": "^2"
1717
}
1818
}

‎tests/Fixtures/packageJson/vendor/symfony/new-package/assets/package.json‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@
1111
},
1212
"entrypoints": ["admin.js"],
1313
"importmap": {
14-
"@hotcake": "^1.9.0",
14+
"@hotcake/foo": "^1.9.0",
1515
"@symfony/new-package": {
16-
"version": "path:%PACKAGE%/dist/loader.js",
17-
"preload": true
16+
"version": "path:%PACKAGE%/dist/loader.js"
1817
}
1918
}
2019
},
2120
"peerDependencies": {
22-
"@hotcookies": "^1.1"
21+
"@hotcookies/bar": "^1.1"
2322
}
2423
}

‎tests/Fixtures/packageJson/vendor/symfony/package-no-file-package/assets/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@
33
"needsPackageAsADependency": false
44
},
55
"peerDependencies": {
6-
"@hotcookies": "^1.1"
6+
"@hotcookies/bar": "^1.1"
77
}
88
}

‎tests/PackageJsonSynchronizerTest.php‎

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Flex\Tests;
1313

14+
use Composer\IO\IOInterface;
1415
use PHPUnit\Framework\TestCase;
1516
use Symfony\Component\Filesystem\Filesystem;
1617
use Symfony\Flex\PackageJsonSynchronizer;
@@ -31,7 +32,8 @@ protected function setUp(): void
3132
$this->synchronizer = new PackageJsonSynchronizer(
3233
$this->tempDir,
3334
'vendor',
34-
$this->scriptExecutor
35+
$this->scriptExecutor,
36+
$this->createMock(IOInterface::class)
3537
);
3638
}
3739

@@ -99,8 +101,8 @@ public function testSynchronizeExistingPackage()
99101
[
100102
'name' => 'symfony/fixture',
101103
'devDependencies' => [
102-
'@hotcookies' => '^1.1|^2',
103-
'@hotdogs' => '^2',
104+
'@hotcookies/bar' => '^1.1|^2',
105+
'@hotdogs/bun' => '^2',
104106
'@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
105107
'@symfony/stimulus-bridge' => '^1.0.0',
106108
'stimulus' => '^1.1.1',
@@ -151,7 +153,7 @@ public function testSynchronizeNewPackage()
151153
'{
152154
"name": "symfony/fixture",
153155
"devDependencies": {
154-
"@hotdogs": "^2",
156+
"@hotdogs/bun": "^2",
155157
"@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
156158
"@symfony/new-package": "file:vendor/symfony/new-package/assets",
157159
"@symfony/stimulus-bridge": "^1.0.0",
@@ -207,8 +209,8 @@ public function testArrayFormattingHasNotChanged()
207209
'{
208210
"name": "symfony/fixture",
209211
"devDependencies": {
210-
"@hotcookies": "^1.1|^2",
211-
"@hotdogs": "^2",
212+
"@hotcookies/bar": "^1.1|^2",
213+
"@hotdogs/bun": "^2",
212214
"@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
213215
"@symfony/stimulus-bridge": "^1.0.0",
214216
"stimulus": "^1.1.1"
@@ -237,8 +239,8 @@ public function testExistingElevatedPackage()
237239
[
238240
'name' => 'symfony/fixture',
239241
'dependencies' => [
240-
'@hotcookies' => '^1.1|^2',
241-
'@hotdogs' => '^2',
242+
'@hotcookies/bar' => '^1.1|^2',
243+
'@hotdogs/bun' => '^2',
242244
'@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
243245
],
244246
'devDependencies' => [
@@ -272,9 +274,9 @@ public function testStricterConstraintsAreKeptNonMatchingAreReplaced()
272274
'name' => 'symfony/fixture',
273275
'devDependencies' => [
274276
// this satisfies the constraint, so it's kept
275-
'@hotcookies' => '^2',
277+
'@hotcookies/bar' => '^2',
276278
// this was too low, so it's replaced
277-
'@hotdogs' => '^2',
279+
'@hotdogs/bun' => '^2',
278280
'@symfony/existing-package' => 'file:vendor/symfony/existing-package/Resources/assets',
279281
],
280282
'browserslist' => [
@@ -303,7 +305,7 @@ public function testSynchronizePackageWithoutNeedingFilePackage()
303305
'{
304306
"name": "symfony/fixture",
305307
"devDependencies": {
306-
"@hotdogs": "^2",
308+
"@hotdogs/bun": "^2",
307309
"@symfony/existing-package": "file:vendor/symfony/existing-package/Resources/assets",
308310
"@symfony/stimulus-bridge": "^1.0.0",
309311
"stimulus": "^1.1.1"
@@ -324,12 +326,13 @@ public function testSynchronizeAssetMapperNewPackage()
324326
$this->scriptExecutor->expects($this->exactly(2))
325327
->method('execute')
326328
->withConsecutive(
327-
['symfony-cmd', 'importmap:require', ['@hotcake@^1.9.0']],
328-
['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath, '--preload']]
329+
['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']],
330+
['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
329331
);
330332

331333
$this->synchronizer->synchronize([
332334
[
335+
// no "importmap" specific config, but still registered as a controller
333336
'name' => 'symfony/existing-package',
334337
'keywords' => ['symfony-ux'],
335338
],
@@ -384,4 +387,55 @@ public function testSynchronizeAssetMapperNewPackage()
384387
json_decode(file_get_contents($this->tempDir.'/assets/controllers.json'), true)
385388
);
386389
}
390+
391+
public function testSynchronizeAssetMapperUpgradesPackageIfNeeded()
392+
{
393+
$importMap = [
394+
'@hotcake/foo' => [
395+
// constraint in package.json is ^1.9.0
396+
'version' => '1.8.0',
397+
],
398+
];
399+
file_put_contents($this->tempDir.'/importmap.php', sprintf('<?php return %s;', var_export($importMap, true)));
400+
401+
$fileModulePath = $this->tempDir.'/vendor/symfony/new-package/assets/dist/loader.js';
402+
$this->scriptExecutor->expects($this->exactly(2))
403+
->method('execute')
404+
->withConsecutive(
405+
['symfony-cmd', 'importmap:require', ['@hotcake/foo@^1.9.0']],
406+
['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
407+
);
408+
409+
$this->synchronizer->synchronize([
410+
[
411+
'name' => 'symfony/new-package',
412+
'keywords' => ['symfony-ux'],
413+
],
414+
]);
415+
}
416+
417+
public function testSynchronizeAssetMapperSkipsUpgradeIfAlreadySatisfied()
418+
{
419+
$importMap = [
420+
'@hotcake/foo' => [
421+
// constraint in package.json is ^1.9.0
422+
'version' => '1.9.1',
423+
],
424+
];
425+
file_put_contents($this->tempDir.'/importmap.php', sprintf('<?php return %s;', var_export($importMap, true)));
426+
427+
$fileModulePath = $this->tempDir.'/vendor/symfony/new-package/assets/dist/loader.js';
428+
$this->scriptExecutor->expects($this->once())
429+
->method('execute')
430+
->withConsecutive(
431+
['symfony-cmd', 'importmap:require', ['@symfony/new-package', '--path='.$fileModulePath]]
432+
);
433+
434+
$this->synchronizer->synchronize([
435+
[
436+
'name' => 'symfony/new-package',
437+
'keywords' => ['symfony-ux'],
438+
],
439+
]);
440+
}
387441
}

0 commit comments

Comments
 (0)