Skip to content
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

Adding support for proxy class generation on container compilations #645

Merged
merged 6 commits into from Feb 28, 2019

Conversation

bbaga
Copy link
Contributor

@bbaga bbaga commented Feb 15, 2019

We've run into an issue where we would have to write custom workaround to pre-generate all proxy classes in our container and it isn't very pretty. Would be much nicer if this functionality would be provided out of the box.

This is my attempt generate the proxies when we build the container.

I wonder if it would be beneficial to put the proxy classes into the container file, like Symfony DI does it.

Issue: #542

Copy link
Member

@mnapoli mnapoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thank you for working on that!

src/Proxy/LazyLoadingValueHolderFactory.php Outdated Show resolved Hide resolved
tests/IntegrationTest/CompiledContainerTest.php Outdated Show resolved Hide resolved
doc/lazy-injection.md Outdated Show resolved Hide resolved
src/Compiler/Compiler.php Outdated Show resolved Hide resolved
src/Compiler/Compiler.php Outdated Show resolved Hide resolved
src/Compiler/ObjectCreationCompiler.php Outdated Show resolved Hide resolved
doc/lazy-injection.md Outdated Show resolved Hide resolved
@mnapoli
Copy link
Member

mnapoli commented Feb 24, 2019

@bbaga I sent a pull request on your pull request 😄 : bbaga#1

The goal was to simplify the changes by removing the custom class that extends ProxyManager. What I did is I invoke the proxy factory instead, we just don't use the created instance. At least we don't deal with ProxyManager internals because I would rather avoid having to update it when new versions are published.

If you merge bbaga#1 we can then merge this one and I can tag a release.

@bbaga
Copy link
Contributor Author

bbaga commented Feb 25, 2019

@mnapoli makes sense, thank you for following up on this! PR merged on my side.

@mnapoli
Copy link
Member

mnapoli commented Feb 28, 2019

Thank you!

@mnapoli mnapoli merged commit f16f27b into PHP-DI:master Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants