-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Preserve placeholder exception for dynamic arrays #29270
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
Conversation
Thinking a bit more about it.. one can only expect env placeholders for string values. Maybe having specialized builder API is the better approach, think; ->ifEnv()
->ifNotEnv()
->ifArrayEnv()
->ifNotArrayEnv() // we need this one for e.g. trusted_hosts instead of isString()
->then() |
@ro0NL for me seems ok. |
@@ -265,6 +265,21 @@ public function testDiscardedEnvInConfig(): void | |||
$this->assertSame('1', $container->getParameter('boolish')); | |||
} | |||
|
|||
/** | |||
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidTypeException | |||
* @expectedExceptionMessage A dynamic value is not compatible with a "Symfony\Component\Config\Definition\PrototypedArrayNode" node type at path "env_extension.nodes". |
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.
should use $this->expectException* now
@ro0NL What's the status here? |
@fabpot it's kinda on hold, but we can discuss further in #27683. Closing as such. What i particularly dont like about this implementation is, each affected node having to handle the case themselves, i.e.;
It hurts maintenance. I prefer something generic that just works, but i dont know how to do it. |
Here's some initial work to deal with envs for array nodes in config.
From #27683 (comment) this would be a first step.
What's still needed is a way to know about the types, e.g. a env resolving to string should be preserved as
array($v)
to be really flexible.cc @stof