Rename MenuDropdown component to MenuItemsDropdown to avoid any case mismatches #19156
Conversation
…ny case mismatches
|
What was the other file that MenuDropdown was in conflict with? Was it in the same folder? And this is a problem during the update? |
|
Ok I saw the related issue, makes sense. |
| @@ -92,7 +92,7 @@ export { default as SideNav } from './SideNav/SideNav'; | |||
| export { default as EnrichedHeadline } from './EnrichedHeadline/EnrichedHeadline.vue'; | |||
| export { default as ContentBlock } from './ContentBlock/ContentBlock.vue'; | |||
| export { default as Comparisons } from './Comparisons/Comparisons.vue'; | |||
| export { default as MenuDropdown } from './MenuDropdown/MenuDropdown.vue'; | |||
There was a problem hiding this comment.
Technically this is a bit of a BC break if a plugin uses MenuDropdown. It could potentially also be exported as MenuDropdown and MenuItemsDropdown.
No plugin probably uses it, though, so it's probably fine.
There was a problem hiding this comment.
@diosmosis but a plugin might need to use the vue component, right? The Angular JS component should still work as before.
I've looked through all our plugins and it wasn't used. I doubt any third party plugin is already migrated to vue...
There was a problem hiding this comment.
Yes a plugin might need to use it which is why changing the export name (MenuDropdown -> MenuItemsDropdown) can be seen as a BC break. It's not important here but worth keeping in mind for the future I think after Matomo 5 when plugins may use them directly.
Description:
Even though most people might have already updated and solved the filename case issue, there could be still people running into that issue if they had installed a version before the rename and update to a newer version.
Simply renaming the component should finally solve that and consume less time than trying to dig longer in the code where it might fail to remove or recreate a file on update when the name only differs in case. We for sure should keep that in mind and should avoid renaming files in case only.
Note: The angularjs adapter still uses the old name as directive, so it can't break any plugins that might still use the old angular js directive. Which is e.g. also the case here:
matomo/plugins/LanguagesManager/templates/getLanguagesSelector.twig
Line 4 in be72457
fixes #18724
Review