Skip to content

Commit

Permalink
bug #769 Fix looking for keywords (nicolas-grekas)
Browse files Browse the repository at this point in the history 
This PR was merged into the 1.12-dev branch.

Discussion
----------

Fix looking for keywords

Fix #768

Commits
-------

9f47b85 Fix looking for keywords
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed May 18, 2021
2 parents 8798024 + 9f47b85 commit 4908784
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 36 deletions.
3 changes: 1 addition & 2 deletions src/Command/GenerateIdCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@

class GenerateIdCommand extends Command
{
public function __construct(/* cannot be type-hinted */ $flex = null)
public function __construct()
{
// No-op to support downgrading to v1.12.x

parent::__construct();
}

Expand Down
8 changes: 4 additions & 4 deletions src/Flex.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,9 @@ private function synchronizePackageJson(string $rootDir)
$synchronizer = new PackageJsonSynchronizer($rootDir, $vendorDir);

if ($synchronizer->shouldSynchronize()) {
$packagesNames = array_column($this->composer->getLocker()->getLockData()['packages'] ?? [], 'name');
$lockData = $this->composer->getLocker()->getLockData();

if ($synchronizer->synchronize($packagesNames)) {
if ($synchronizer->synchronize($lockData['packages'] ?? []) || $synchronizer->synchronize($lockData['packages-dev'] ?? [])) {
$this->io->writeError('<info>Synchronizing package.json with PHP packages</>');
$this->io->writeError('<warning>Don\'t forget to run npm install --force or yarn install --force to refresh your JavaScript dependencies!</>');
$this->io->writeError('');
Expand Down Expand Up @@ -774,8 +774,8 @@ public function updateAutoloadFile()
file_put_contents($autoloadFile, <<<EOPHP
<?php
if (\PHP_VERSION_ID < $version) {
echo sprintf("Fatal Error: composer.lock was created for PHP version $platform or higher but the current PHP version is %d.%d.%d.\\n", PHP_MAJOR_VERSION, PHP_MINOR_VERSION, PHP_RELEASE_VERSION);
if (PHP_VERSION_ID < $version) {
echo sprintf("Fatal Error: composer.lock was created for PHP version $platform or higher but the current PHP version is %d.%d.%s.\\n", PHP_MAJOR_VERSION, PHP_MINOR_VERSION, PHP_RELEASE_VERSION);
exit(1);
}
$code
Expand Down
3 changes: 0 additions & 3 deletions src/InformationOperation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ public function getPackage()
return $this->package;
}

/**
* {@inheritdoc}
*/
public function getJobType()
{
return 'information';
Expand Down
29 changes: 15 additions & 14 deletions src/PackageJsonSynchronizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ public function shouldSynchronize(): bool
return $this->rootDir && file_exists($this->rootDir.'/package.json');
}

public function synchronize(array $packagesNames): bool
public function synchronize(array $phpPackages): bool
{
// Remove all links and add again only the existing packages
$didAddLink = $this->removePackageJsonLinks((new JsonFile($this->rootDir.'/package.json'))->read());

foreach ($packagesNames as $packageName) {
$didAddLink = $this->addPackageJsonLink($packageName) || $didAddLink;
foreach ($phpPackages as $phpPackage) {
$didAddLink = $this->addPackageJsonLink($phpPackage) || $didAddLink;
}

$this->registerPeerDependencies($packagesNames);
$this->registerPeerDependencies($phpPackages);

// Register controllers and entrypoints in controllers.json
$this->registerWebpackResources($packagesNames);
$this->registerWebpackResources($phpPackages);

return $didAddLink;
}
Expand All @@ -76,14 +76,14 @@ private function removePackageJsonLinks(array $packageJson): bool
return $didRemoveLink;
}

private function addPackageJsonLink(string $phpPackage): bool
private function addPackageJsonLink(array $phpPackage): bool
{
if (!$packageJson = $this->resolvePackageJson($phpPackage)) {
return false;
}

$manipulator = new JsonManipulator(file_get_contents($this->rootDir.'/package.json'));
$manipulator->addSubNode('devDependencies', '@'.$phpPackage, 'file:'.substr($packageJson->getPath(), 1 + \strlen($this->rootDir), -13));
$manipulator->addSubNode('devDependencies', '@'.$phpPackage['name'], 'file:'.substr($packageJson->getPath(), 1 + \strlen($this->rootDir), -13));

$content = json_decode($manipulator->getContents(), true);

Expand Down Expand Up @@ -112,10 +112,11 @@ private function registerWebpackResources(array $phpPackages)
if (!$packageJson = $this->resolvePackageJson($phpPackage)) {
continue;
}
$name = '@'.$phpPackage['name'];

foreach ($packageJson->read()['symfony']['controllers'] ?? [] as $controllerName => $defaultConfig) {
// If the package has just been added (no config), add the default config provided by the package
if (!isset($previousControllersJson['controllers']['@'.$phpPackage][$controllerName])) {
if (!isset($previousControllersJson['controllers'][$name][$controllerName])) {
$config = [];
$config['enabled'] = $defaultConfig['enabled'];
$config['fetch'] = $defaultConfig['fetch'] ?? 'eager';
Expand All @@ -124,13 +125,13 @@ private function registerWebpackResources(array $phpPackages)
$config['autoimport'] = $defaultConfig['autoimport'];
}

$newControllersJson['controllers']['@'.$phpPackage][$controllerName] = $config;
$newControllersJson['controllers'][$name][$controllerName] = $config;

continue;
}

// Otherwise, the package exists: merge new config with uer config
$previousConfig = $previousControllersJson['controllers']['@'.$phpPackage][$controllerName];
$previousConfig = $previousControllersJson['controllers'][$name][$controllerName];

$config = [];
$config['enabled'] = $previousConfig['enabled'];
Expand All @@ -145,7 +146,7 @@ private function registerWebpackResources(array $phpPackages)
}
}

$newControllersJson['controllers']['@'.$phpPackage][$controllerName] = $config;
$newControllersJson['controllers'][$name][$controllerName] = $config;
}

foreach ($packageJson->read()['symfony']['entrypoints'] ?? [] as $entrypoint => $filename) {
Expand Down Expand Up @@ -191,11 +192,11 @@ public function registerPeerDependencies(array $phpPackages)
file_put_contents($this->rootDir.'/package.json', $manipulator->getContents());
}

private function resolvePackageJson(string $phpPackage): ?JsonFile
private function resolvePackageJson(array $phpPackage): ?JsonFile
{
$packageDir = $this->rootDir.'/'.$this->vendorDirname.'/'.$phpPackage;
$packageDir = $this->rootDir.'/'.$this->vendorDirname.'/'.$phpPackage['name'];

if (!\in_array('symfony-ux', json_decode(file_get_contents($packageDir.'/composer.json'), true)['keywords'] ?? [], true)) {
if (!\in_array('symfony-ux', $phpPackage['keywords'] ?? [], true)) {
return null;
}

Expand Down
5 changes: 0 additions & 5 deletions tests/Fixtures/packageJson/vendor/symfony/existing-package/composer.json
View file

This file was deleted.

5 changes: 0 additions & 5 deletions tests/Fixtures/packageJson/vendor/symfony/new-package/composer.json
View file

This file was deleted.

25 changes: 22 additions & 3 deletions tests/PackageJsonSynchronizerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ public function testSynchronizeNoPackage()

public function testSynchronizeExistingPackage()
{
$this->synchronizer->synchronize(['symfony/existing-package']);
$this->synchronizer->synchronize([
[
'name' => 'symfony/existing-package',
'keywords' => ['symfony-ux'],
],
]);

// Should keep existing package references and config
$this->assertSame(
Expand Down Expand Up @@ -107,7 +112,16 @@ public function testSynchronizeExistingPackage()

public function testSynchronizeNewPackage()
{
$this->synchronizer->synchronize(['symfony/existing-package', 'symfony/new-package']);
$this->synchronizer->synchronize([
[
'name' => 'symfony/existing-package',
'keywords' => ['symfony-ux'],
],
[
'name' => 'symfony/new-package',
'keywords' => ['symfony-ux'],
],
]);

// Should keep existing package references and config and add the new package, while keeping the formatting
$this->assertSame(
Expand Down Expand Up @@ -159,7 +173,12 @@ public function testSynchronizeNewPackage()

public function testArrayFormattingHasNotChanged()
{
$this->synchronizer->synchronize(['symfony/existing-package']);
$this->synchronizer->synchronize([
[
'name' => 'symfony/existing-package',
'keywords' => ['symfony-ux'],
],
]);

// Should keep existing array formatting
$this->assertSame(
Expand Down

4 comments on commit 4908784

@spu95
Copy link

@spu95 spu95 commented on 4908784 May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We got an fatal error:
16:10:41 PHP Fatal error: Uncaught TypeError: Argument 1 passed to Symfony\Flex\PackageJsonSynchronizer::addPackageJsonLink() must be of the type string, array given, called in .../src/vendor/symfony/flex/src/PackageJsonSynchronizer.php on line 41 and defined in .../src/vendor/symfony/flex/src/PackageJsonSynchronizer.php:66

@nicolas-grekas
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you have a global installation of flex? Please let me know how that's possible. By looking at the code, I can't figure out.

@nicolas-grekas
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #770

@spu95
Copy link

@spu95 spu95 commented on 4908784 May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually by looking in the code I cant figure it out too.. We had to update the dependencies more than one time to make it works..

Please sign in to comment.