-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI][FrameworkBundle] Add PSR-11 "ContainerBag" to access parameters as-a-service #25288
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…as-a-service
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,8 @@ | |
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; | ||
use Symfony\Component\DependencyInjection\Exception\LogicException; | ||
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader; | ||
use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface; | ||
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface; | ||
use Symfony\Component\EventDispatcher\EventSubscriberInterface; | ||
|
@@ -110,6 +112,12 @@ public function load(array $configs, ContainerBuilder $container) | |
$loader->load('services.xml'); | ||
$loader->load('fragment_renderer.xml'); | ||
|
||
if (!interface_exists(ContainerBagInterface::class)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not just bumping the min version of the DI component instead ? This is a hard requirement already. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because I don't want to be the one breaking the 3-cross-4 compat :) |
||
$container->removeDefinition('parameter_bag'); | ||
$container->removeAlias(ContainerBagInterface::class); | ||
$container->removeAlias(ParameterBagInterface::class); | ||
} | ||
|
||
if (class_exists(Application::class)) { | ||
$loader->load('console.xml'); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\DependencyInjection\ParameterBag; | ||
|
||
use Symfony\Component\DependencyInjection\Container; | ||
|
||
/** | ||
* @author Nicolas Grekas <p@tchwork.com> | ||
*/ | ||
class ContainerBag extends FrozenParameterBag implements ContainerBagInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could probably be made final, I see no reason to extend this rather than implementing the interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't make classes final "by default", that's not the framework's policy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's good idea. Could that
F438
change for new classes? |
||
{ | ||
private $container; | ||
|
||
public function __construct(Container $container) | ||
{ | ||
$this->container = $container; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function all() | ||
{ | ||
return $this->container->getParameterBag()->all(); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function get($name) | ||
{ | ||
return $this->container->getParameter($name); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function has($name) | ||
{ | ||
return $this->container->hasParameter($name); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\DependencyInjection\ParameterBag; | ||
|
||
use Psr\Container\ContainerInterface; | ||
use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException; | ||
|
||
/** | ||
* @author Nicolas Grekas <p@tchwork.com> | ||
*/ | ||
interface ContainerBagInterface extends ContainerInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we move this one namespace up? is typehinting in general #24738 needs There was a problem hiding this comment. 10000Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
that's a parameter bag, should be in the namespace to me
yes: the new interface is more tailored than, the existing one, better for autosuggestion by IDEs
isn't that already what the new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah but why favor
would do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's say you have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I was thinking you'd prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait... do the point of this interface is just for DX? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there anything we do in a framework that is not for DX? |
||
{ | ||
/** | ||
* Gets the service container parameters. | ||
* | ||
* @return array An array of parameters | ||
*/ | ||
public function all(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return type declaration? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not possible: would make the interface incompatible with ParameterBagInterface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm too late but, is this true? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it isn't. |
||
|
||
/** | ||
* Replaces parameter placeholders (%name%) by their values. | ||
* | ||
* @param mixed $value A value | ||
* | ||
* @throws ParameterNotFoundException if a placeholder references a parameter that does not exist | ||
*/ | ||
public function resolveValue($value); | ||
|
||
/** | ||
* Escape parameter placeholders %. | ||
* | ||
* @param mixed $value | ||
* | ||
* @return mixed | ||
*/ | ||
public function escapeValue($value); | ||
|
||
/** | ||
* Unescape parameter placeholders %. | ||
* | ||
* @param mixed $value | ||
* | ||
* @return mixed | ||
*/ | ||
public function unescapeValue($value); | ||
} |
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.
access
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.
will fix directly