Add "skipDestructor" option to lazy loading proxies #641
Conversation
|
About the mutation score, I tried the mutants locally, they all fail when applied. I don't know what is missing, please advise. |
| $excludedMethods = ProxiedMethodsFilter::DEFAULT_EXCLUDED; | ||
|
|
||
| if ($skipDestructor) { | ||
| $excludedMethods[] = '__destruct'; |
There was a problem hiding this comment.
Nit: rename the option to "excludeDestructor" since that seems to be the internal wording.
There was a problem hiding this comment.
Not sure about this proposal: "exclude" here is used for a different thing.
There was a problem hiding this comment.
My thinking was that you're excluding the destructor from the list of proxied methods, but I defer to your (and @Ocramius') judgment on this 👍
|
Potentially, this could also be a solution to #468 and #373, and giving the end user the foot-gun seems like a sensible choice. Patch makes sense, although I do need to take more time to review. Be aware that this will only land in a new minor release, which may end up being |
src/ProxyManager/ProxyGenerator/LazyLoadingValueHolderGenerator.php
Outdated
Show resolved
Hide resolved
|
No time to get it in |
|
Rebased, let me know the next iteration. |
This PR adds a new
skipDestructoroption to lazy proxies (ghosts and value holders) that skips calling the original destructor when the object is not initialized.This is something that is much needed in DI containers, where e.g. we don't want opening a database connection on an unused Doctrine object.
For the record, Symfony already has a similar check in place, but it is implemented using monkey-patching. Because of the policies in place here (new features are available only on the very latest PHP version), I had absolutely no incentive in contributing here since I couldn't actually use that work anytime soon.
Now that I decided to maintain a fork of this code, I know that anything I contribute here could be used in the short term. That's a great incentive.
The good news is that you don't have to change your policies. OSS dynamics FTW here also.