Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[OptionsResolver] Deprecate defining nested options viasetDefault() usesetOptions() instead#59618

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

Merged
fabpot merged 1 commit intosymfony:7.3fromyceruto:options_resolver_nested
Feb 26, 2025

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedJan 26, 2025•
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?yes
Issues-
LicenseMIT

this removes unnecessary limitations that I hadn't considered when introducing nested options feature in#27291.

1. Allow defining default values for nested options

Imagine you want to define the following nested option in a generic form type:

class GenericTypeextends AbstractType{publicfunctionconfigureOptions(OptionsResolver$resolver):void    {$resolver->setDefault('foo',function (OptionsResolver$foo) {$foo->define('bar')->allowedTypes('int');        });    }}

then, I'd like to define a concrete type with a default value for it.

class ConcreteTypeextends AbstractType{publicfunctionconfigureOptions(OptionsResolver$resolver):void    {$resolver->setDefault('foo', ['bar' =>23]);    }publicfunctiongetParent():string    {return GenericType::class;    }}

this might seem unexpected, but the fact is that the nested definition forfoo option inConcreteType is gone. As a result, when resolved, thefoo option will have a default value (['bar' => 23]) but without any constraints, allowing end users any value/type to be passed through this option forConcreteType instances

For example, passing['foo' => false] as options forConcreteType would be allowed, instead of requiring an array wherebar expects an integer value.

2. Allow defining lazy default for nested options

the same problem would occur with a lazy default for a nested definition:

class ConcreteTypeextends AbstractType{publicfunctionconfigureOptions(OptionsResolver$resolver):void    {$resolver->setRequired(['baz'])$resolver->setDefault('foo',function (Options$options) {return ['bar' =>$options['baz']];        });    }publicfunctiongetParent():string    {return GenericType::class;    }}

the issue here is the same as in the previous example, meaning this new default essentially overrides/removes the original nested definition

the two features mentioned earlier are now supported by introducing a new methodsetOptions(), which separates the nested definition from its default value (whether direct or lazy). Additionally this PR deprecates the practice of defining nested options usingsetDefault() method

this also enables the ability to set default values for prototyped options, which wasn't possible before. For example:

class NavigatorTypeextends AbstractType{publicfunctionconfigureOptions(OptionsResolver$resolver):void    {$resolver->define('buttons')            ->options(function (OptionsResolver$buttons) {$buttons->setPrototype(true);$buttons->define('name')->required()->allowedTypes('string');$buttons->define('type')->default(SubmitType::class)->allowedTypes('string');$buttons->define('options')->default([])->allowedTypes('array');             })            ->default(['back' => ['name' =>'back','options' => ['validate' =>false,'validation_groups' =>false]],'next' => ['name' =>'next'],'submit' => ['name' =>'submit'],            ]);    }}

cheers!

@yceruto
Copy link
MemberAuthor

Psalm failure is a false positive.
Other errors are not related

@ycerutoycerutoforce-pushed theoptions_resolver_nested branch from616e8fc to48d601fCompareJanuary 26, 2025 16:20
@ycerutoycerutoforce-pushed theoptions_resolver_nested branch from48d601f to042cdfeCompareJanuary 28, 2025 17:53
@ycerutoyceruto changed the title[OptionsResolver] Deprecate defining nested options viasetDefault() usesetNested() instead[OptionsResolver] Deprecate defining nested options viasetDefault() usesetOptions() insteadJan 28, 2025
@ycerutoycerutoforce-pushed theoptions_resolver_nested branch 2 times, most recently from877a720 to0ad4b89CompareJanuary 31, 2025 19:02
@ycerutoycerutoforce-pushed theoptions_resolver_nested branch 3 times, most recently from3ed6e2d tode64012CompareFebruary 4, 2025 15:39
@ycerutoycerutoforce-pushed theoptions_resolver_nested branch fromde64012 to1172bdaCompareFebruary 5, 2025 17:00
@ycerutoycerutoforce-pushed theoptions_resolver_nested branch from1172bda tob0bb9a1CompareFebruary 20, 2025 14:18
@yceruto
Copy link
MemberAuthor

Just fixing conflicts

@fabpot
Copy link
Member

Thank you@yceruto.

@fabpotfabpot merged commitffdbc83 intosymfony:7.3Feb 26, 2025
8 of 11 checks passed
@ycerutoyceruto deleted the options_resolver_nested branchFebruary 26, 2025 12:34
fabpot added a commit that referenced this pull requestMar 1, 2025
…ebugCommand` (yceruto)This PR was merged into the 7.3 branch.Discussion----------[Form] Add support for displaying nested options in `DebugCommand`| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | -| License       | MITAddressing#59618 (comment)```bash$ bin/console debug:form FooType baz --format=json```being `baz` a nested option of the `FooType`, output example in JSON format:```json{    "required": false,    "default": [],    "is_lazy": false,    "has_normalizer": false,    "has_nested_options": true,    "nested_options": {        "foo": {            "required": true,            "default": true,            "is_lazy": false,            "has_normalizer": false,            "has_nested_options": false        },        "bar": {            "required": false,            "default": true,            "is_lazy": false,            "has_normalizer": false,            "has_nested_options": false        }    }}```The new additions here are the `has_nested_options` and `nested_options` entries.Cheers!Commits-------2a6d2d4 Add support for displaying nested options in DebugCommand
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMar 19, 2025
…`setOptions` instead of `setDefault` (yceruto)This PR was merged into the 7.3 branch.Discussion----------[OptionsResolver] Update nested options examples to use `setOptions` instead of `setDefault`closes#20705PRsymfony/symfony#59618Commits-------159a150 Update nested options examples to use setOptions instead of setDefault
@fabpotfabpot mentioned this pull requestMay 2, 2025
nicolas-grekas added a commit that referenced this pull requestJun 6, 2025
…finition via `setDefault()`, use `setOptions()` instead (yceruto)This PR was merged into the 8.0 branch.Discussion----------[OptionsResolver] Remove support for nested options definition via `setDefault()`, use `setOptions()` instead| Q             | A| ------------- | ---| Branch?       | 8.0| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | -| License       | MITRef:#59618Commits-------f1abd5c Remove support for nested options definition via `setDefault()`, use `setOptions()` instead
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.3

Development

Successfully merging this pull request may close these issues.

5 participants

@yceruto@fabpot@nicolas-grekas@stof@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp