To use PHP-DI 6.0 with the Symfony Bridge, with Symfony 3.3, 3.4 et 4+ #19
Conversation
src/Kernel.php
Outdated
| @@ -105,7 +108,7 @@ private function disableDebugClassLoader() | |||
| protected function getPHPDIContainer() | |||
| { | |||
| if ($this->phpdiContainer === null) { | |||
| $builder = new \DI\ContainerBuilder(); | |||
| $builder = new DiContainerBuilder(); | |||
There was a problem hiding this comment.
For clarity I'd rather not alias the class (and the namespace is really short anyway), could you restore that?
src/Kernel.php
Outdated
| @@ -37,9 +38,11 @@ public function __construct($environment, $debug) | |||
| /** | |||
| * Implement this method to configure PHP-DI. | |||
| * | |||
| * @param DiContainerBuilder $builder | |||
| * | |||
There was a problem hiding this comment.
This parameter is not bringing value, could you revert?
src/SymfonyContainerBridge.php
Outdated
| @@ -24,7 +24,7 @@ | |||
| * | |||
| * @author Matthieu Napoli <[email protected]> | |||
| */ | |||
| class SymfonyContainerBridge extends SymfonyContainer implements SymfonyContainerInterface, ContainerInterface | |||
| class SymfonyContainerBridge extends SymfonyContainer implements SymfonyContainerInterface | |||
There was a problem hiding this comment.
This removal is because the interface is already implemented in Symfony right?
Is it possible to keep it anyway (for future-proofing)?
There was a problem hiding this comment.
Yes, I remove it because it already implemented in Symfony from the version 3.3.
But, it's allowed by PHP (no issue on an internal project with PHP 7.1), but some tools (PHPStorm...) display a warning.
We can also restore the original interface (Interop\Container\ContainerInterface instead of the PSR11) to avoid a BC break. (SF implements the PSR11).
There was a problem hiding this comment.
No it's good to target PSR-11 only. So if I understand correctly can you add:
implements SymfonyContainerInterface, ContainerInterface
(ContainerInterface would be PSR-11)
| $phpdiContainer->set('foo', \DI\get('class2')); | ||
| $ref = \DI\get('class2'); | ||
| $ref->setName('foo'); | ||
| $phpdiContainer->set('foo', $ref); |
There was a problem hiding this comment.
Did you have to do this? Because it looks like a regression in PHP-DI 6 to me 🤔
There was a problem hiding this comment.
Yes, for me its a regression in PHP-DI 6, the \DI\get returns a reference and not an helper, and so the entry name will never injected into it. So the name is empty. (In PHP-DI 5 its an helper).
But I use PHP-DI only since few months, (it'is a pretty tool). So my experience with this project is limited and I did not have time to see if that was the expected behavior or not for PHP-DI 6.
There was a problem hiding this comment.
Thank you I have created an issue to check this in PHP-DI itself. It should not block this PR though.
|
Thanks for your review. I will fix them this week. |
|
It's done. Sorry, it was a busy week.. |
|
Thanks! |
Hi,
It's some patchs to allow use PHP-DI 6 with the bridge on Symfony 3.3, 3.4 and 4+.
This patch set the minimum required version of PHP to 7.0 (needed by PHP-DI 6).
I limited to Symfony 3.3 and higher because, from this version its container follows the PSR-11.
To allow alias from the PHP-DI container to the Symfony container, the Symfony service must be tagged as public, else it's not available with SF 4.0 and a deprecation notice is thrown with Symfony 3.4.
Have a nice day.
Richard.