[Workflow] Don't use VariableNode for ArrayNode #39328
Conversation
| @@ -384,13 +384,9 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode) | |||
| ->defaultValue([]) | |||
| ->prototype('scalar')->end() | |||
| ->end() | |||
| ->variableNode('events_to_dispatch') | |||
| ->defaultValue(null) | |||
| ->arrayNode('events_to_dispatch') | |||
There was a problem hiding this comment.
an arrayNode must use either ->children or prototype for it to work fine (otherwise, it will be an array node using children() but with no allowed children, which is useless)
There was a problem hiding this comment.
Btw, once you change that to a proper prototype, the validate function can then be moved to the prototype instead of looping over the array, which would report the invalid value to a more precise location
|
@Nyholm your implementation forbids setting |
|
Hello, We wanted something a bit complexe here. We want to be able to
This is why we ended with the current code. IIRC, we have tests for this. So If you achieve to make the code simpler, without having to edit the fixtures, it will be nice :) When I finished #37815, I search for simpler code (may be not enought) but I can not find a better way. |
|
I see. Thank you for the context and the review. I was dumping configuration reference and saw that the |
|
Im closing this in favor of #39336 |
…r VariableNode with array example (Nyholm) This PR was merged into the 4.4 branch. Discussion ---------- [Config] YamlReferenceDumper: No default value required for VariableNode with array example | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | | Tickets | | License | MIT | Doc PR | This will fix #39328 in a better way. A `VariableNode` may have an array as value and also a default value as "null". (Like the workflow component). When we dump the config, we generate invalid yaml: ``` workflows: enabled: false workflows: # Prototype name: # Select which Transition events should be dispatched for this Workflow events_to_dispatch: null # Examples: - workflow.enter - workflow.transition ``` With this PR, we will remove the `null` default value from the dumped config. #SymfonyHackday Commits ------- 9104fd4 [Config] YamlReferenceDumper: No default value required for VariableNode with array example
This change could break some applications that have defined:
They should just remove the
events_to_dispatchline or define it as an empty array.I don't see why it was defined as a VariableNode in the first place. It was introduced here #37815, maybe @lyrixx can help me here?
#SymfonyHackday