laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

[Proposal] Automatic discovering service providers #541

Closed vyuldashev closed 7 years ago

vyuldashev commented 7 years ago

Hello,

The problem

Each time I install package I need to manually register service providers. This looks like a redundant step.

The solution

Laravel can auto discover service providers. For example, there is a library called Discovery: https://github.com/thecodingmachine/discovery Laravel can use it or we can develop out own implementation of discovering service providers.

How it works Every package required to have discovery.json:

{
  "Illuminate\\Support\\ServiceProvider": [
    {
      "value": "PackageVendor\PackageName\PackageServiceProvider"
    }
  ]
}

Every time we run composer install or install new package - a folder .discovery will be created, which will contain discovered service providers. It's fast.

Finally, we can use this approach not only for loading service providers, but also for loading views, migrations, etc.

barryvdh commented 7 years ago

We should probably look at how Symfony 4 will be handling this: http://fabien.potencier.org/symfony4-workflow-automation.html

tomschlick commented 7 years ago

I disagree. I see having to add the Service Provider to app.php as a feature.

It allows you explicitly state what should be running and in what order. It also allows you to filter out certain providers depending on the environment (by moving them to their own sub providers).

I don't think a 20 second copy/paste is that much of a burden for the amount of magic that would be needed to automatically do this.

crynobone commented 7 years ago

What happen if a package has separate service provider for Laravel and Lumen?

What happen if you want to only have a package required by certain environment?

What happen if you want to extends package service provider because you need to extends some services?

vyuldashev commented 7 years ago

@crynobone

What happen if a package has separate service provider for Laravel and Lumen? What happen if you want to only have a package required by certain environment?

It can be defined in metadata.

{
   "Illuminate\\Support\\ServiceProvider":[
      {
         "value":"PackageVendor\\PackageName\\PackageServiceProvider",
         "metadata":{
            "frameworks":[
               "laravel",
               "lumen"
            ],
            "environments":[
               "testing",
               "local"
            ]
         }
      }
   ]
}

What happen if you want to extends package service provider because you need to extends some services?

I never found myself extending service providers. But, may be for that purpose, we can still may have an option to define providers in config/app.php, which will look if that ones overrides any of the vendor service providers.

brunogaspar commented 7 years ago

Each time I install package I need to manually register service providers. This looks like a redundant step.

Doesn't look redundant, it looks like you're just lazy to do a simple copy/paste.

I usually do this when Composer is installing the dependency so it's a win/win since i don't waste much time when Composer is doing it's thing and most of the time i can still look at the composer status lol

What happen if a package has separate service provider for Laravel and Lumen?

What happen if you want to only have a package required by certain environment?

What happen if you want to extends package service provider because you need to extends some services?

All very good points from @crynobone

--

My opinion on this

Personally, not a fan of this kind of automation as in this particular situation it will most likely create more issues than really solving anything as it doesn't even seem flexible enough.

It sure does removes a very very small and super fast step that is to add the service provider to the config/app.php file..but the beauty of Laravel is that it doesn't get in your way, you can setup the application the way you want, use whatever you want whenever you want without needing to touch endless files.

Your proposal to use this discovery package (or maybe build a similar one) is going to remove AN EASY THING and overcomplicate it with the addition of json files.

Also, package developers will then have an extra task which is to create an unnecessary manifest.json file which seems to mimic what the service provider already does?

This is obviously just my opinion.

It's a 👎 from me.

vyuldashev commented 7 years ago

@brunogaspar

I am not talking about how heavy this step (adding service provider). I am talking about it look like it's redundant. Because I already installed a package through composer and I need to install it again by adding it's service provider to configuration file. Why just don't install packages your application really needs? This also works for queues, events and other things. If my application needs queue functionality I just do composer require illuminate/queue and it works!

Laravel stands for easiness and, in my opinion, this king of improvement will add much more beauty and will thin your application to what it really needs and does.

tomschlick commented 7 years ago

Composer doesn't install anything, it downloads it. Adding the service provider is the install step.

michaeldyrynda commented 7 years ago

I don't think this adds anything of value and ultimately, it's just a micro-optimisation. For the amount of time it might save and the number of times you're actually adding service providers, it's unnecessary overhead.

Dylan-DPC-zz commented 7 years ago

you can use https://github.com/patinthehat/laravel-require if you want this functionality

mikebronner commented 7 years ago

If I have a situation where I know I want to add many service providers to a project (or all my projects) I have created package bundles. That allows me to specify the service provider of that package, which loads all my desired packages in the order and manner they need to be in. This lets you quickly and simply bootstrap an app without having to deal with all the hassle of adding 10s or packages each time.

I agree with @michaeldyrynda that the configuration and mental debt this incurs is more than the effort required at this time. I don't necessarily agree that this request comes out of laziness, but can see that you are probably looking at this in an effort to slip-stream your development process. You could write a package that does this, or use the one you referenced to do this, but it probably shouldn't be baked into the core without first addressing the issues others have brought up.

Admittedly, we might not be seeing your use-case. If that's the case, please elaborate on your original post with more details on the real problem it solves -- adding auto-discovery doesn't solve a problem, it just adds a feature that may or may not be useful. What specific problem do we struggle with now that is tedious or difficult to implement?

decadence commented 7 years ago

https://github.com/laravel/framework/pull/19420 Can be closed

vyuldashev commented 7 years ago

Yes! I am happy that this feature finally arrives.