-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix new service replacement service provider precedence on core factory implementations #4593
Fix new service replacement service provider precedence on core factory implementations #4593
Conversation
@MGatner what about |
The calls themselves are okay since all instances will be stored in
...otherwise when Core fails to match an instance and calls |
Also, if you could find a way to add tests that confirm the correct instance is loaded I think that would be a great addition. |
@MGatner sadly I'm not able to do this (adding tests) by my own right now. |
No worries, these tests were already lacking and that's not your fault. I will take a look at them later. For now can you change |
@MGatner it's done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay! I thought I submitted this but I see this morning that it is "pending". One small style change and we're golden!
07b3f0c
to
4339db0
Compare
Thanks 👍 |
Hi, Before this change, it was possible to create different Service classes to categorize the services. A developer only needed to create a new class (inherit from But after this, Why you didn't write Thanks |
Basically because this would ignore the service provider precedence, see #4483 If youre against this behavior, you can easily extend @MGatner @paulbalandan what about (auto) service-provider discovery? Anything planned in that direction? CodeIgniter4/system/Config/BaseService.php Line 313 in 9d23926
|
Thank you @element-code for your answer. 👍 |
This fixes #4483
Replacing
static::foobar()
with\Config\Services::foobar()
(https://regex101.com/r/QyahlU/1).Checklist: