Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Config] Allow to always use config classes#51544
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Config] Allow to always use config classes#51544
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedSep 29, 2023
Thanks for working on this, it's related to the limitation I found in#51273 |
nicolas-grekas left a comment
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.
The changes on the Config components are BC breaks. Can we find a way that doesn't break BC?
| if ($this->default) { | ||
| if (!\is_array($this->defaultValue)) { | ||
| thrownew \InvalidArgumentException(sprintf('%s: the default value of an array node has to be an array.',$node->getPath())); | ||
| thrownew \InvalidArgumentException(sprintf('"%s": the default value of an array node has to be an array.',$node->getPath())); |
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 be reverted, that's a false-positive from fabbot I suppose
| protected$node; | ||
| public$allowedTypes; |
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.
removing consts and public properties is a BC break, we need to figure out a way that doesn't break BC
- undo changes fromsymfony#44166
f4bb242 to72e1fd0Compare| $parameterTypes =$this->getParameterTypes($node); | ||
| $comment .=' * @param ParamConfigurator|'.implode('|',$parameterTypes).' $value'."\n"; | ||
| $parameterType =$this->getParameterType($node); | ||
| if (null ===$parameterType ||'' ===$parameterType) { |
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.
why returning an empty string to meanmixed instead of returningmixed ?
| * @template TValue | ||
| * @param TValue $value | ||
| * @return \Symfony\Config\NodeInitialValues\MessengerConfig|$this | ||
| * @psalm-return (TValue is array ? \Symfony\Config\NodeInitialValues\MessengerConfig : static) |
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.
why should passing an array return a MessengerConfig while passing another kind of value would be fluent ? What does this correspond to ?
nicolas-grekasSep 19, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
I wondered about the same and I think I've got the answer:
- When you pass a non-array, you likely won't want to configure anything else nested in the tree you're configuring. That's what happens for other config formats, structurally.
- When you pass an array, you can configure nested options, so that then it might make sense to return the nested config-builder.
I'm just wondering about the check: should we return the config-builder when the array is non empty?
Or should we do this?
| * @psalm-return (TValue is array ? \Symfony\Config\NodeInitialValues\MessengerConfig : static) | |
| * @psalm-return (TValue isnon-empty-array ?$this :\Symfony\Config\NodeInitialValues\MessengerConfig) |
| * @return $this | ||
| */ | ||
| publicfunctionkeyedArray(string$name,ParamConfigurator|string|array$value):static | ||
| publicfunctionkeyedArray(string$name,mixed$value):static |
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.
this type becoming wider looks like a regression to me.
nicolas-grekas commentedSep 26, 2025
Let me close as stalled and superseded by#51273 |
When the config node doesn't have normalization, using the config object is impossible.
When the config node has some array normalization (like
http_codesin\Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration::addHttpClientRetrySection), it is not possible to use the config object.This PR relates to changes introduced in: