NomisCZ / flarum-ext-auth-steam

Flarum Steam Login extension
https://discuss.flarum.org/d/19750-steam-auth
MIT License
6 stars 6 forks source link

Guzzle requirement missing from composer.json #12

Closed clarkwinkelmann closed 4 years ago

clarkwinkelmann commented 4 years ago

This extension uses guzzlehttp/guzzle but does not explicitly requires it in composer.json. This will cause errors if users have no other extensions that require guzzlehttp/guzzle.

NomisCZ commented 4 years ago

Hello, thanks for the info. I'll check it.

NomisCZ commented 4 years ago

Guzzle is required by Laravel, so is not necessary to explicitly require it.

clarkwinkelmann commented 4 years ago

Actually, it isn't.

Flarum does not require the laravel/framework or laravel/laravel package directly, but instead directly requires some of the individual Laravel packages, none of them require Guzzle.

Laravel only added Guzzle as a requirement to laravel/laravel in a later version, but this has no influence on Flarum because we don't use the Laravel scaffolding.

composer why guzzlehttp/guzzle will show you which other dependencies actually require Guzzle on your forum. No Laravel packages are in there.

You can reproduce the issue by removing all other social login extensions and Upload with Composer. You'll see Guzzle is no longer installed.

Even if one of your dependencies had Guzzle as a dependency (which is not the case here), it's best practice to explicitly require it yourself as that dependency could drop Guzzle.

My report follows the findings discussed at https://discuss.flarum.org/d/23034-flarum-query-extensions-big-data-analysis/2