Actually I just realized that theusort function is not stable :(

http://www.php.net/manual/en/function.usort.php

"If two members compare as equal, their relative order in the sorted array is undefined."

This means we would have to sort differently to be 100% sure there is no BC break?

What about using thePriorityTaggedServiceTrait ? (or at least a\SplPriorityQueue like other passes like theAddSecurityVotersPass orDecoratorServicePass ones ?)

It seems that also\SplPriorityQueue will not preserve the initial order of elements if the priority is equal (meaning its not a "stable sort"):

http://php.net/manual/de/class.splpriorityqueue.php#117067

@dmaicher There is no stable sorting by default either as it depends on the order of the services registration (which is not predictable unless using a custom pass, not sure it is worth it in that case though), and this is partially responsible for the issue you're trying to solve.

I don't see why you shouldn't use the trait.

Currently form type extensions cannot rely on other extensions, at least handling priorities will solve this.

@HeahDude But as you said currently without priorities it depends on the order of service registration. If we introduce a non-stable sorting then it might be a BC break though as the order of service registration is not 100% guaranteed to be used anymore for the form extensions (when all extensions have equal priority 0)?

currently without priorities it depends on the order of service registration

What I'm saying is that this order is not reliable already, so it is an acceptable BC break IMO because it does not change anything unless some extensions rely on each other, which is buggy right now unless using your new feature :)

@HeahDude Ok now I see what you mean 😊 If this "BC break" is acceptable then I can surely change it 😄

Other opinions on that?

You can still usethe trait as inspiration and maybe play around with the existing function.

I changed it to use\SplPriorityQueue now 😉

If all agree this feature is useful as is I could prepare a PR for the docs?

Relying on the order is it really something we want to support? Do we have enough real-world use cases? Nobody ever had this issue in the last 5 years, so I'm wondering if we really need to support this.

Form type extensions are meant to decouple specific configuration, thus may add options to resolve. So on one hand I agree, this is maybe covering edge cases where one need to handle that kind of option globally.
But on the other hand, the logical solution is to use an extension and in such cases having a full control of the order has a significant importance, otherwise it's just unpredictable.

Actually resolving the form configuration is why we need this composite pattern, and the process works well because it is mainly based on inheritance which do keep an order, so if we can support it for extensions too, I guess we should.

Good things:

• DX: user is in full control (workaround from@javiereguiluz is good but not enough, and neither ismine, the option may be undefined andthrow an exception or implies todefine it first which is definitely not user friendly),
• Perf: it is handled in the compiler pass so no runtime overhead,
• Design: this is consistent with the composite pattern: options, form and view are resolved from the higher parent to lowest priority extension,

Bad things:

• This is not a bug fix, so master only :troll face:

Anyway thank you@dmaicher for proposing this feature.

I would also agree with@HeahDude. I don't see any disadvantages of having this really small (+10, -3 LOC excluding tests) feature. Sure maybe not so many people will use it but currently the behaviour is a bit unpredictable for the order of form type extensions which is quite bad imho 😢

@dmaicher After re-reading this, I still think you should use the trait and change onlythis line, you can then keep the same logic as before to handle the indexation by extended type but you'll create the priority job only once which will also use only one instance oneSplPriorityQueue since anyway by default no extension will have a priority set (keeping the same old unpredictable behavior ;).

I think that is is a good thing too because in the future if there is some issue about extensions overriding each other we will be able to solve it thanks to this feature.

@HeahDude

with this

foreach ($this->findAndSortTaggedServices('form.type_extension',$container)as$reference) {...}

I just have a sorted list of serviceReference objects. How do I get theextended_type info then from the tags?

But I think you are right about sorting all extensions globally though 😊 Might make more sense.

@dmaicher : TheReference instance converted to string gives you the service id. Just get the definition back from it, and access the tags :)

LGTM Thanks@dmaicher :)

So what the conclusion here? ping@symfony/deciders

@dmaicher If this is accepted you'll have to update the framework bundle changelog file as well.

@dmaicher can you please update the CHANGELOG file of the component?

@nicolas-grekas done 😉 Let me know if I need to change anything else

@dmaicher Can you please update the description too (tests are passing now :) and submit a PR in the docs? Thanks again!

@HeahDude done 😉

👍

Status: Reviewed

Thank you@dmaicher.

A bit late to the party here, but I absolutely think this was a BC break. Seedunglas/DunglasAngularCsrfBundle#39

The entire bundle's csrf protection form extension is broken because it now loads before the core csrf extension -- it disables csrf on a form in favor of its own system only to have it re-enabled by the core extension which now loads later in 3.2.

@chrisguitarguy this should have been fixed with#20995

Great! Will have to downgrade to Symfony 3.1 until the next bugfix release of 3.2, though.