Add ability to swap core models for own models
Is your feature request related to a problem? Please describe. It's not easy to add functionality to models. Yes, we have macros but that feels cumbersome. Also, macroable models are discouraged by the Laravel team so it kind of feels not right to use them.
Describe the solution you'd like I would like to see that we could easily swap core models for our own. This could be done in a config (Spatie does this for medialibrary classes) or serviceprovider.
Describe alternatives you've considered I've tried to extend core models, but that did not work because in some parts of the code the core models/namespaces are expected.
This is something that has been requested a few times. The macroable functionality was a stop-gap solution.
I think it's something we need to look at, but it's not at the top of our list right now.
If someone wants to create a draft PR with a suggested approach, I would be interested to see it.
Thanks for considering this 🙏🏼
I think it could be just something like
// config.php
'models' => [
'product_model' => MyPackage\Models\MyProduct::class,
'cart_model' => GetCandy\Models\Cart::class,
'collection_model' => GetCandy\Models\Collection::class,
// ...
]
// Reference it anywhere
config('getcandy.models.product_model')::find(1);
but maybe @adam-code-labx could also share his take on it? 😎
Be good to see how some other packages do it. I feel like the must be a tidier way.
Also, I don't want to have to list out every single model in config, only the ones a developer wants to override.
Maybe we need a little helper function/class.
Be good to see how some other packages do it. I feel like the must be a tidier way.
I think so too yeah. But this is how Spatie does it for their medialibrary package.
Yeah, they only have a single model to worry about though. We have quite a lot!
Thinking maybe something like
gc_model('Product')::find(1);
Where the helper function gc_model() looks to see if anything is set in config, if not it assumes the standard models directory.
Also, I don't want to have to list out every single model in config, only the ones a developer wants to override.
Maybe we need a little helper function/class.
I somewhat agree. Personally I think a developer should have complete freedom to decide what needs overriding and not be limited.
Maybe we need a way for addons to register their own models too, so they too can be overridden using the same technique?
Sure @gavinhewitt I can see your point with setting the models in the config but also see @glennjacobs point having to add all models in this way will certainly cause breaking changes not to mention zero IDE completion.
One idea I have which I will look further into next week which would mean no breaking changes in GC and complete developer flexibility is.
- Add trait to BaseModel something like hasExtendedModel
This trait will check the config for the model namespace as it could be App or even an Addon with the ability to extend the model, this new trait will have it's own boot method with the ability to call the matched extended model for example
App\Models\ProductExtendedwhere you will be able to add your own methods or override existing ones
Well thats my initial idea need to make a few tests, unless of course the GC team beat me to it 😃 I would propose the same for single action classes although we don't have too many of these right now. Used this package for a few projects recently https://laravelactions.com/ 😉
We've used the Laravel Actions package before, in fact it was used in v1 of GetCandy. However, we decided against it for v2.
One idea I have which I will look further into next week which would mean no breaking changes in GC and complete developer flexibility is.
- Add trait to BaseModel something like hasExtendedModel This trait will check the config for the model namespace as it could be App or even an Addon with the ability to extend the model, this new trait will have it's own boot method with the ability to call the matched extended model for example
App\Models\ProductExtendedwhere you will be able to add your own methods or override existing ones
Sounds quite complex in comparison. I'm not sure if this would introduce issues. The only way to find out I guess is to knock something up.
We've used the Laravel Actions package before, in fact it was used in v1 of GetCandy. However, we decided against it for v2.
Curious why you decided against it for V2
We've used the Laravel Actions package before, in fact it was used in v1 of GetCandy. However, we decided against it for v2.
Curious why you decided against it for V2
We are quite strict on the third-party packages we use for GetCandy now. Part of our mission is to approach GetCandy in the most "Laravel way" as possible. Almost like "What would Taylor do?".
So packages must provide a lot of value and be well supported for us to consider them. Laravel Actions completely changed in v2, meaning we couldn't upgrade easily and it would have been a big chore to convert, so I don't want that again.
We don't see enough value in Laravel Actions to incorporate it.
Yeah, they only have a single model to worry about though. We have quite a lot!
Thinking maybe something like
gc_model('Product')::find(1);Where the helper function
gc_model()looks to see if anything is set in config, if not it assumes the standard models directory.
The issue for me with this one is IDE completion I suppose we can inline but prefer my solution will put something together.
/** @var \Models\Product $product */
$product = gc_model('Product')::find(1);
or even create factory class then cleaner implementation to extend or swap from the service container or even from an Addon ServiceProvider ModelFactory::make('Product')->extend(Product::class);
Yeah, a factory class is what I had in my head. So you can register the model overrides from anywhere and then easily load the correct one.
Maybe it could be that you put something like this in your AppServiceProvider...
use GetCandy\Facades\ModelFactory;
use App\Models\Product;
ModelFactory::register('Product', Product::class);
And then to use the model, something like
use GetCandy\Facades\ModelFactory;
ModelFactory::resolve('Product')::find(1);
Could even add autoloading namespaces, e.g.
use GetCandy\Facades\ModelFactory;
ModelFactory::addNamespace('GetCandy\Core\Models');
Would just need to consider priority of loading if more than one namespace added.
Yeah it could work but seems a lot more work for you guys to change the codebase especially all the model relations. Adding a trait on the BaseModel which will auto register will perhaps be the simplest approach.
I imagine the ModelFactory would not be high priority for GC at this stage as there would be a lot of work to swap out every model and update all the unit tests in order to resolve the new model class?
That is exactly it, swapping all the models out would be a big chore, hence we're not doing it right now.
If the trait can work, it might be an option.
@glennjacobs @gavinhewitt Will look into putting together this temporary magical model trait this weekend and create my 4th PR 😂 Have a great weekend.