Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Add hasPackage method to Composer class #48124

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

emargareten
Copy link
Contributor

This PR adds a hasPackage method as a continuation of #48096, such a method is also being used in the starter kits

@emargareten emargareten changed the title [10.x] Add hasPackage method to Composeer class [10.x] Add hasPackage method to Composer class Aug 20, 2023
@crynobone
Copy link
Member

crynobone commented Aug 20, 2023

We can actually use Composer Runtime API which is required by Laravel Framework 10 and that's why it not added in the previous PR:
https://getcomposer.org/doc/07-runtime.md#knowing-whether-package-x-or-virtual-package-is-present

@emargareten
Copy link
Contributor Author

@crynobone I copied this implemetation from breeze (same thing in jetstream), I think using the Composer Runtime API in this scenario might not return the expected results since it checks for all sub-dependecies while we need to check the top level dependencies only.

@crynobone
Copy link
Member

this scenario might not return the expected results since it checks for all sub-dependecies while we need to check the top level dependencies only.

Have you confirm this claim?

@emargareten
Copy link
Contributor Author

see https://github.com/laravel/breeze/blob/1.x/src/Console/InstallCommand.php#L76C1-L79C14

if ($this->option('pest') || $this->isUsingPest()) {
    if ($this->hasComposerPackage('phpunit/phpunit')) {
        $this->removeComposerPackages(['phpunit/phpunit'], true);
    }

    // ...
}

We are checking if phpunit exists in top level deps

@taylorotwell taylorotwell merged commit 66b8ac1 into laravel:10.x Aug 21, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants