Skip to content

[OptionsResolver] Deprecate defining nested options via setDefault() use setOptions() 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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 26, 2025
Merged

[OptionsResolver] Deprecate defining nested options via setDefault() use setOptions() instead #59618

merged 1 commit into from
Feb 26, 2025

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jan 26, 2025 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? yes
Issues -
License MIT

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 GenericType extends AbstractType
{
    public function configureOptions(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 ConcreteType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefault('foo', ['bar' => 23]);
    }

    public function getParent(): string
    {
        return GenericType::class;
    }
}

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

For example, passing ['foo' => false] as options for ConcreteType would be allowed, instead of requiring an array where bar 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 ConcreteType extends AbstractType
{
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setRequired(['baz'])
        $resolver->setDefault('foo', function (Options $options) {
            return ['bar' => $options['baz']];
        });
    }

    public function getParent(): 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 method setOptions(), which separates the nested definition from its default value (whether direct or lazy). Additionally this PR deprecates the practice of defining nested options using setDefault() method

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

class NavigatorType extends AbstractType
{
    public function configureOptions(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!

@carsonbot carsonbot added this to the 7.3 milestone Jan 26, 2025
@yceruto yceruto force-pushed the options_resolver_nested branch 2 times, most recently from 5aee060 to 616e8fc Compare January 26, 2025 05:28
@yceruto
Copy link
Member Author

yceruto commented Jan 26, 2025

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

@yceruto yceruto force-pushed the options_resolver_nested branch from 616e8fc to 48d601f Compare January 26, 2025 16:20
@yceruto yceruto force-pushed the options_resolver_nested branch from 48d601f to 042cdfe Compare January 28, 2025 17:53
@yceruto yceruto changed the title [OptionsResolver] Deprecate defining nested options via setDefault() use setNested() instead [OptionsResolver] Deprecate defining nested options via setDefault() use setOptions() instead Jan 28, 2025
@yceruto yceruto force-pushed the options_resolver_nested branch 2 times, most recently from 877a720 to 0ad4b89 Compare January 31, 2025 19:02
@yceruto yceruto force-pushed the options_resolver_nested branch 3 times, most recently from 3ed6e2d to de64012 Compare February 4, 2025 15:39
@yceruto yceruto force-pushed the options_resolver_nested branch from de64012 to 1172bda Compare February 5, 2025 17:00
@yceruto yceruto force-pushed the options_resolver_nested branch from 1172bda to b0bb9a1 Compare February 20, 2025 14:18
@yceruto
Copy link
Member Author

yceruto commented Feb 20, 2025

Just fixing conflicts

@fabpot
Copy link
Member

fabpot commented Feb 26, 2025

Thank you @yceruto.

@fabpot fabpot merged commit ffdbc83 into symfony:7.3 Feb 26, 2025
8 of 11 checks passed
@fabpot fabpot mentioned this pull request Feb 26, 2025
Open
@yceruto yceruto deleted the options_resolver_nested branch February 26, 2025 12:34
@yceruto yceruto mentioned this pull request Feb 27, 2025
Merged
fabpot added a commit that referenced this pull request Mar 1, 2025
@fabpot
feature #59871 [Form] Add support for displaying nested options in `D…
959bbc6 
…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       | MIT

Addressing #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
@yceruto yceruto mentioned this pull request Mar 4, 2025
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Mar 19, 2025
@javiereguiluz
minor #20719 [OptionsResolver] Update nested options examples to use …
1da928a 
…`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 #20705
PR symfony/symfony#59618

Commits
-------

159a150 Update nested options examples to use setOptions instead of setDefault
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof left review comments

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Deprecation Feature OptionsResolver Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
5 participants
@yceruto @fabpot @nicolas-grekas @stof @carsonbot