-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[OptionsResolver] Support nested options #4833
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
Comments
@bschussek Any news about this? |
@blaugueux Not yet. Maybe this can be tackled in 2.2. |
@bschussek if you can guide me thru this I could help out and implement the changes, seeing this is a long standing issue. Thanks! |
@bschussek ping on this issue. @dlsniper Have you done any work on this? |
@mtotheikle sorry, I didn't got any answer from @bschussek and currently I'm pretty booked as well for at least one more month. If you can do it, go ahead :) Regards. |
If this hasn't been started I may be able to look at it over the next week or so, but would need some ideas for how it should be implemented. Any ideas @dlsniper? |
@bschussek does OptionResolver really need this feature? Should be great but can remove the simplicity of the component too. After a year or so, what is your opinion about this PR? |
I think it would be useful. There is a few array options in the form component which would benefit from it |
I agree with both of you. It's useful, but it's hard to implement while still keeping everything simple. |
We can give a try, we'll see if things can keep simple. Otherwise the produced code would be re arranged in another component with OptionsResolver as a dependency for example. |
any news on this feature? I'm have a similar problem with the attr option in the symfony2 forms, default attr options get overwriten by attributes set in the formConfig |
One thing I can think of at this moment is to add $optionsResolver = new OptionsResolver();
$nestedOptionsResolver = new OptionsResolver();
$optionsResolver->setNestedOption(array('option_nested' => $nestedOptionsResolver)); When an value with key 'option_nested' is passed to One minor detail is parent dependency, as some options are lazy evaluated with a Closure they/may should have access to the parent options resolver to check some values for the final value. And compatibility, we cant expend the Interface as this a BC, so we should either introduce an additional interface or come-up with a with way to reuse the current interface. |
I'm new to symfony and the open source community, so I'm not certain about protocol here. And I don't fully understand how this resolver works. But I would very much like to see setDefaults work on nested arrays, so that I can pass in options that are grouped in a sane way (rather than all on the same single level), e.g.
Currently OptionsResolver#resolve() does a foreach() through the options and does a set() which calls Options#overload. So why doesn't overload check if an option is an array and do an array_replace_recursive? |
@chadwickmeyer is not not only about defaults. It is also about all the logic resolving options. And we cannot simply make it recurse when it finds an array. This would break existing cases where options are accepting arrays as values (for instance |
Thanks for responding. If I knew the whole system better, I would love to offer my help. But I am not qualified yet to understand the big picture (which is required to implement this feature). For the time being I found another way to do the merging in my application, and then I send all the defaults and the custom values to the form builder, since they have already been merged in my application . |
closes symfony#4833. Allow to resolve nested options.
closes symfony#4833. Allow to resolve nested options.
closes symfony#4833. Allow to resolve nested options.
closes symfony#4833. Allow to resolve nested options.
closes symfony#4833. Allow to resolve nested options.
closes symfony#4833. Allow to resolve nested options. * add `OptionsResolver::nested` * add `OptionsResolver::setNested()` * add `OptionsResolver::isNested()` * add `OptionsResolver::getNested()`
This might be overly simplistic, but wouldn't it suffice to automatically resolve an
The only bc-break this would introduce is that an I actually have a (seemingly) working implementation for the above (minus access to the parent resolver), though I wanted to first ask if this approach would be acceptable at all? |
I will close this issue. It's been open for a while and I don't think that this is something that the OptionsResolver really should support. I also don't see a way to do this without making the implementation overly complex. |
For all interested: I've found this package sauls/options-resolver which does support deep options resolving by |
See #27291 for full implementation of the initial request. Could this request be reopened, please? |
…finition (yceruto) This PR was merged into the 4.2-dev branch. Discussion ---------- [OptionsResolver] Added support for nesting options definition | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #4833 | License | MIT | Doc PR | symfony/symfony-docs#9995 I'd like to propose an alternative to #27251 and #18134 with a different approach. It would allow you to create a nested options system with required options, validation (type, value), normalization and more. <details> <summary><strong>Short documentation</strong></summary> **To define a nested option, you can pass a closure as the default value of the option with an `OptionsResolver` argument:** ```php $resolver ->defaults([ 'connection' => 'default', 'database' => function (OptionsResolver $resolver) { $resolver ->setRequired(['dbname', 'host', ...]) ->setDefaults([ 'driver' => 'pdo_sqlite', 'port' => function (Options $options) { return 'pdo_mysql' === $options['driver'] ? 3306 : null, }, 'logging' => true, ]) ->setAllowedValues('driver', ['pdo_sqlite', 'pdo_mysql']) ->setAllowedTypes('port', 'int') ->setAllowedTypes('logging', 'bool') // ... }, ]); $resolver->resolve(array( 'database' => array( 'dbname' => 'demo', 'host' => 'localhost', 'driver' => 'pdo_mysql', ), )); // returns: array( // 'connection' => 'default', // 'database' => array( // 'dbname' => 'demo', // 'host' => 'localhost', // 'driver' => 'pdo_mysql', // 'port' => 3306, // 'logging' => true, // ), //) ``` Based on this instance, you can define the options under ``database`` and its desired default value. **If the default value of a child option depend on another option defined in parent level, adds a second ``Options`` argument to the closure:** ```php $resolver ->defaults([ 'profiling' => false, 'database' => function (OptionsResolver $resolver, Options $parent) { $resolver ->setDefault('logging', $parent['profiling']) ->setAllowedTypes('logging', 'bool'); }, ]) ; ``` **Access to nested options from lazy or normalize functions in parent level:** ```php $resolver ->defaults([ 'version' => function (Options $options) { return $options['database']['server_version']; }, 'database' => function (OptionsResolver $resolver) { $resolver ->setDefault('server_version', 3.15) ->setAllowedTypes('server_version', 'numeric') // ... }, ]) ; ``` As condition, for nested options you must to pass an array of values to resolve it on runtime, otherwise an exception will be thrown: ```php $resolver->resolve(); // OK $resolver->resolve(['database' => []]); // OK $resolver->resolve(['database' => null); // KO (Exception!) ``` </details> --- Demo app https://github.com/yceruto/nested-optionsresolver-demo Commits ------- d04e40b Added support for nested options definition
It is currently not possible to configure nested options. For example:
It is not possible to resolve this configuration in the following way:
because now the other default values under "db" will be dropped. Also, it is not possible to specify allowed values or types for the nested options.
The text was updated successfully, but these errors were encountered: