RTC PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar HLeithner
HLeithner
16 Jan 2025

Summary of Changes

Set the namespace in componentHelper::getComponent() result.

Testing Instructions

Use an namespace extension, execute var_dump(componentHelper::getComponent('com_xxx');

Actual result BEFORE applying this Pull Request

$namespace is always null

Expected result AFTER applying this Pull Request

$namespace is filled if extension has an namespace

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar HLeithner HLeithner - open - 16 Jan 2025
avatar HLeithner HLeithner - change - 16 Jan 2025
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Jan 2025
Category Libraries
avatar HLeithner HLeithner - change - 16 Jan 2025
Labels Added: PR-5.2-dev
avatar joomdonation joomdonation - test_item - 18 Jan 2025 - Tested successfully
avatar joomdonation
joomdonation - comment - 18 Jan 2025

I have tested this item ✅ successfully on 98907ee


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44737.

avatar rdeutz rdeutz - test_item - 18 Jan 2025 - Tested successfully
avatar rdeutz
rdeutz - comment - 18 Jan 2025

I have tested this item ✅ successfully on 98907ee


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44737.

avatar rdeutz rdeutz - change - 18 Jan 2025
Status Pending Ready to Commit
avatar rdeutz
rdeutz - comment - 18 Jan 2025

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44737.

avatar richard67 richard67 - change - 18 Jan 2025
Labels Added: RTC
avatar Fedik
Fedik - comment - 18 Jan 2025

I am sorry to say, but this should not be merged.
With this we pulling few extra bites of manifest_cache for all components on each request for nothing.

$namespace is always null

It is design flaw. Can just remove that as useles property :)

If we need this feature it should be proper API that will cover all extension.
IDK, namespace column in #__extensions or something to read it from extension service provider.

avatar rdeutz
rdeutz - comment - 18 Jan 2025

I am not sure why it is needed but it is one field more in the queue that is already running and it sets a propery that is also there. So if this is not needed then we should remove the property and not let it NULL.

avatar HLeithner
HLeithner - comment - 18 Jan 2025

that's the point an empty variable doesn't make sense and in this I need it so I thought it was simply for forgotten.

avatar joomdonation joomdonation - alter_testresult - 18 Jan 2025 - joomdonation: Not tested
avatar joomdonation
joomdonation - comment - 18 Jan 2025

Sorry, I was aware that we do not use it in core at the moment, but didn't think carefully enough about few extra bytes on each request for nothing which @Fedik point out. For this, I agree with @Fedik


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44737.

avatar HLeithner
HLeithner - comment - 18 Jan 2025

Ok, then please tell me a better solution to get a namespace of any extension.

avatar Fedik
Fedik - comment - 18 Jan 2025

please tell me a better solution to get a namespace of any extension

In theory we should not need to know the namespace, however I agree in some cases it is usefull.
As for now I have no idea.
Maybe kind of $app->bootExtension('potato')->getNamespace() which is aslo bad because that is doing whole extension initialisation.

If I would need it I would just do some little helper for myself that read it directly from XML, as it done in https://github.com/joomla/joomla-cms/blob/5.2-dev/libraries/namespacemap.php

Or maybe as part of JNamespacePsr4Map kind of (new JNamespacePsr4Map)->getNamespace('component', 'com_potato')

avatar HLeithner
HLeithner - comment - 18 Jan 2025

ok then I see we have a function what exactly does your suggestion but you don't want it because creating a completely new (everyone at it's own) is better?

Sorry but that doesn't make sense for me. Even if you are right that the namespace in theory is not needed, but only if you are in the joomla context, is a bit different if you do strange things like prefixing 3rd party libraries.

To solve my issue I think I would only need a helperFactory in the component but to be honest that's something which is even worse ;-)

avatar Fedik
Fedik - comment - 18 Jan 2025

Well, that is not me who designed it ;)
We cannot even get extension service container and that even more confusing than unable to access to its namespace.

I think someone just forgot to remove the $namspace property of ComponentRecored when JNamespacePsr4Map was writen.

avatar HLeithner
HLeithner - comment - 18 Jan 2025

I'm ok with deprecating this variable for the future if we have a better way but at the moment I see no other way coming to town right?

avatar Hackwar Hackwar - change - 18 Jan 2025
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2025-01-18 20:59:43
Closed_By Hackwar
avatar Hackwar Hackwar - close - 18 Jan 2025
avatar Hackwar Hackwar - merge - 18 Jan 2025
avatar Hackwar
Hackwar - comment - 18 Jan 2025

I'm siding with Harald on this one and thus merged it. Thank you! If you think this is wrong, lets please open a PR to deprecate this attribute.

avatar Fedik
Fedik - comment - 19 Jan 2025

What a horrible mistake you guys just did. In patch release.

avatar laoneo
laoneo - comment - 20 Jan 2025

Please guys, revert this pr. It brings us back to the ages where single classes will be instantiated. @Fedik is completely right, this needs to be removed and everything should go through the service provider.

avatar pe7er
pe7er - comment - 21 Jan 2025

I've re-added the 5.2.4 milestone. We've added this to the 5.2-dev branch for 5.2.4, and fixed it with #44755

Add a Comment

Login with GitHub to post a comment