Laravel-Backpack / community-forum

A workspace to discuss improvement and feature ideas, before they're actually implemented.
28 stars 0 forks source link

[Proposal] Change recommended way of extending Backpack Addon classes #90

Open tabacitu opened 4 years ago

tabacitu commented 4 years ago

As mentioned here on twitter... I'm now wondering if we should make the way @lambasoft suggested the recommended way to extend add-ons in the future...

So instead of creating a routes file, that basically overwrites all routes, and changing the controller it points to, we would do:

carbon (1)

This will make Laravel load that one class, instead of that one other class from the package.


The way I see it:

PROs:

CONs:


But I guess we can work around the first CON by also suggesting following a convention:

And we could work around the second CON (maybe, not sure this is better in any way) by not doing that inside the AppServiceProvider at all. We could instead provide a new config inside config/backpack/base.php, where developers could specify "replace X with Y" for all Backpack or Backpack-related classes. But... wouldn't this be even more obscure? 👀

// Use your own classes instead of the ones provided by Backpack, or by a Backpack addon.
// Reads as "instead of class X, use class Y".
'extended_classes' => [
    \VendorName\PackageName\Http\Controllers\ExampleController::class => \App\Http\Controllers\ExampleControllerExtended,
],

What's definitely cool is that we could even automate the above. We could (or maybe should) create a command, something like php artisan backpack:extend Path\To\Class\Name that would:

That would make it super-easy to extend something from a package. Maybe even... too easy? 🤣

@lambasoft , @DoDSoftware , @pxpm what do you think? Anybody else have thoughts on this?

pxpm commented 4 years ago

I do think that the way we extend something depends on what we want to do in the end.

Bind classes this way, makes the developer copy paste the whole class, while the traditional method: 'MyNewController' extends 'OldController' and point the route to the new controller instead of the old one, allow us to only override what we really need (one function or whatever) in the new controller.

Maybe a big change like "hooks" in all backpack functionality would make wonders.

What I think is like: setupListOperation has a "hook" that's called setupListOperationHook that we call internally on backpack, so if dev do in App\Provider

SomePackage\EntityController::macro('setupListOperationHook' function() {
// $this = CrudPanel::class;
$this->addField(['name' => 'new_field_in_controlller_setup_operation']);
}

That way people don't even need to override controllers for simple functions like adding a field, removing save actions etc ...

Let me know, Pedro

DoDSoftware commented 4 years ago

I like this train of thought a lot. @tabacitu if I've understood correctly, we would still use the traditional method: 'MyNewController' extends 'OldController' and MyNewController could still only overwrite some properties and methods selectively, this is really just about informing Backpack of which controller the request gets routed to, right?

pxpm commented 3 years ago

100% right @DoDSoftware I misread it.

But I still like hooks! \o :)

DoDSoftware commented 3 years ago

@pxpm Indeed! I love your hook suggestion for cases where we might want to just add a field, column, or button etc without having to overwrite the whole controller. I definitely think there's a place for both