deprecated-packages / symplify

[DISCONTINUED] Check split packages in their own repositories :)
MIT License
619 stars 188 forks source link

[monorepo-builder] Code example in documentation is misleading #4391

Open tobalsan opened 2 years ago

tobalsan commented 2 years ago

Hi,

in the monorepo builder package's README.md file, the source code provided for the monorepo-builder.php file is a bit misleading.

The comment says:

// what extra parts to add after merge?

but in reality, the main method used by the vendor/bin/monorepo-builder merge command actually performs the decorations (adds, replaces, and removals) prior to merging all packages' composer.json files, not after:

public function createFromRootConfigAndPackageFileInfos(ComposerJson $mainComposerJson, array $packageFileInfos) : void
{
    // This applies any modification we specified in the `monorepo-builder.php` file using Option::DATA_TO_APPEND or Option::DATA_TO_REMOVE
    $mergedAndDecoratedComposerJson = $this->mergePackageFileInfosAndDecorate($packageFileInfos);
    // Then it merges the composer.json files afterward
    $this->composerJsonMerger->mergeJsonToRoot($mainComposerJson, $mergedAndDecoratedComposerJson);
}

I don't know if this is by design or not, but when looking at the README.md file I thought I could update specific parts of the final composer.json, after all sub-composer.json were merged. More precisely, I wanted to remove certain parts of it that are added during the merge, but it's not possible since the removal happens before the merge.

Adding extra stuff works because the merge doesn't remove what's been added while in the decorator process. But I wanted to remove a package from the replace section of the root composer.json, and this doesn't work because it gets re-added every time during the merge.

TomasVotruba commented 2 years ago

Hi, thank you for your description. What would you suggest to make it better?

tobalsan commented 2 years ago

Easy / lazy solution: update the comment in the code example so it says that JSON manipulation happens BEFORE the merge.

Real solution of course is to process the removals on the final, merged root composer.json. I could make a PR if it's something you think is worth doing (I found a workaround for my personal need).

TomasVotruba commented 2 years ago

I could make a PR if it's something you think is worth doing

That would be amazing :+1: