Should we really support both ways to solve it?

Yes I think so, if one wants to override choices, he should not be forced to override the loader.

It makes no sense IMO to enforce lazy load something that is already loaded, if you have your choices, just pass them, do not encapsulate them in an objet that will eventually call a function to return the initial thing.

@xabbuh supporting a single way to achieve it would actually involvemore work to forbid one of them. Both ways are just features of the ChoiceType: ifchoice_loader is not null, it is used. Otherwisechoices is used.

The CountryType & co currently have to do extra work to ensure that thischoices wins over thedefault choice_loader, due to BC (there was no default choice loader previously).

Either we should only support thechoice_loader solution in the future or talk about the possibility to explicitly set it tonull here too.

I hesitated, considering your comments I'd rather add it explicitly here too, thanks.

Done, I've just updated the notices.

Using the "choices" option [...]

Fixed everywhere.

Do we really need the type extension here?

I would keep it like it for consistency with the test above, and because it test both overriding the option directly and doing it with an extension, so looks safer as is.

I think we need to reword this a bit to make clear that this is only deprecated as long as you do not set thechoice_loader option tonull (something like "using thechoices option without overriding thechoice_loader option is deprecated").

Good idea! Done.

That doesn't look intuitive to me.

Well, this is because choice_loader has priority over choices in ChoiceType (which is not documented IIRC btw)

And this should not be used ideally, the ChoiceType is more accurate in such cases, I'll update the doc to clarify this.

Wouldn't it be "better" to drop this example and just keep the second code snippet? And remove the recommendation to usechoice_loader as null?

I understand this does not look pretty, but this is the actual quick fix for some breaking app.

What we can't do is to stop supporting "choices" option as it's inherited from the ChoiceType, and in such casesnull is needed because of its precedence.

What we can do is to better document these types options, and explicit that "choices" or "choice_loader" options should be overridden on a ChoiceType or an EntityType only, not on a child already doing so, unless the previous value is reused.

I would like to address your comment, but as I said in a previous outdated diff:

It makes no sense IMO to enforce lazy load something that is already loaded, if you have your choices, just pass them, do not encapsulate them in an objet that will eventually call a function to return the initial thing.

Then the real fix would be to use the ChoiceType (if the previous "choices" values is not used), I could add this instead, but objectively I would let it as is now.

I'm waiting for your final call on this symfony deciders.

I am fine with how it is now. But we should indeed clarify the documentation about the differences of thechoices andchoice_loader options and when to use what.