Fragment priority when reusing the same identifier #6471
Comments
|
I always assumed the order of bundles affects the sorting as well as the priority, simply because the array that stored the information must have an order. However, due to sorting algorithms, that could actually be lost. Regarding the command list, would you still get two text elements when priority is used? That looks like a bug to me. |
Yes, because there are still two independent services. And for each of them we add calls for the fragment registry and the debug command. If there are two services with the same type, we should IMHO either only add method calls for the latter (which would solve the problem in the debug command) or keep the behavior but then store the data in the command differently or index it by identifier, so that only the effective one is shown. After all, in the the registry, there is a comment suggesting, that it's fine to add multiple calls with the same identifier: contao/core-bundle/src/Fragment/FragmentRegistry.php Lines 19 to 25 in a084e49 |
|
By the way, currently the debug command would just output identical rows, because it iterates the identifiers but looks up the wrong data. 😄 |
|
The problem only occurs in 5.x |
Description ----------- Preparation PR to fix the issue #6471 afterwards. Commits ------- 9e49973 Use dedicated fragment registry for debug:fragments command 13dc311 Adjust tests afd98f8 Refactor a5d730f Update core-bundle/src/Command/DebugFragmentsCommand.php fb1f24a Loop directly iterate over keys and values
Affected version(s)
5.3
Description
Currently, there are two issues when redeclaring a content element/frontend module type.
If you want to replace the
textcontent element, you could for instance do something like this:But the core's
texttype would still be used. Additionally thedebug:fragmentscommand would output two entries fortextbut both would show the core's class.The reason for this is, that the
RegisterFragmentPassuses priority tagged services, i.e. you need to manually tag your service with apriority(it does not work in the attribute). I would argue, that we should probably use the bundle loading order instead or - at least - the app should always come last by default if not configured differently.The issue with
debug:fragmentsis something else: there theadd()method assumes it is only called once per identifier. We should probably store all information together (data[] = […, …, …]) instead of in three different arrays.As this whole setup is compile time only, we could also change the logic in the
RegisterFragmentPassto only add the effective calls to the definitions once all found entities were processed./cc @bytehead
The text was updated successfully, but these errors were encountered: