Namespace JHtml

[laoneo]

Summary of Changes

This is a backport of pr #17328 to Joomla 3. Additionally JHtml got namespaced and for demonstration purposes JHtmlJquery. If this one gets accepted, then I will namespace the rest of the services as well in another pr.

Namespaces JHtml, why not the registry, se comments below.

Testing Instructions

• Open the back end
• Check the source code of the page

Expected result

All is working as expected.

Actual result

All is working as expected.

[mbabker]

1. Why does the class need to be renamed HTMLHelper?
2. Why is this not backported with the registry class using the same name as it was accepted in?
3. Migrating things to services is B/C breaking because if it exists as a service then it doesn't go into the class/path lookup, the only thing that still works is if you're using a plugin based approach to overload the services before their first calls. Hence the reason in my PR I purposefully didn't migrate the core library helpers for 4.0.

As is, this is not a backport, as it contains changes from the accepted PR introducing the new API.

[laoneo]

1. How would you name it then. Html did sound for me wrong.
2. Because we gave every class in libraries meaningful names. Registry is very general one which we have already in use.
3. Changed in e72d8e2 the HTMLHelper class the way that it checks the paths first. To test it, I copied the jquery file from staging into the folder administrator/components/com_content/helpers/html and add the code \Joomla\CMS\HTML\HTMLHelper::addIncludePath(JPATH_ADMINISTRATOR.'/components/com_content/helpers/html'); in the stats plugin at the beginning of onAfterInitialise. If you add then a breakpoint (or die) in the function framework in the jquery.php file in com_content, then it executed it from the override. So we should be BC safe or?

[mbabker]

Because we gave every class in libraries meaningful names. Registry is very general one which we have already in use.

Part of the point of namespaces is that the fully qualified class name provides the full meaning. Not every short class name must duplicate the preceding namespace segment, it's really only heavily used in places where the short name on its own could cause confusion because it can be duplicated across namespaces (i.e. does "Articles" refer to a table, model, controller). If you're going to insist on renaming the class though, ServiceRegistry is a better option because that's what the class actually is.

I purposefully did not migrate the core helpers in libraries/cms/html to use the service registry because it has major B/C implications with the lookup methods. Very explicitly, my aim was that if a service provider/vendor opts to register their helper to the registry, that should take precedence over path lookups. By putting the registry lookup after the path lookups, it will break services if the same class is resolved through path lookups and the class does not use static methods.

Therefore, the core helpers MUST NOT be migrated to use the service registry until 5.0 when all of the existing method registration/overloading logic is removed. Extension helpers may safely migrate to the service registry.

[laoneo]

Therefore, the core helpers MUST NOT be migrated to use the service registry until 5.0

Here I don't agree. This would lead to a hard BC break, so why not offering that functionality right now in 3 already in a BC way? I think it is easier to move the condition up in the code afterwards for 5 than namespacing all the classes.

[mbabker]

You cannot migrate the core libraries to being service driven without an internal B/C break, no matter how you try to restructure JHtml::_() and making the service registry lookup come after the file path lookup defeats the purpose of introducing the service registry to begin with. The design of the transitional API is very simple:

1. Keys registered via JHtml::register() (either externally by plugins or internally after a method is resolved) (this is deprecated)
2. The service registry (which relaxes some of the class naming and structure logic the existing methodology has; one can overload a service and extend a single method while keeping the parent class in place whereas the path lookup logic requires you to duplicate the entire class)
3. The path lookup logic (this is deprecated)

By flipping the ordering of 2 and 3 around, you get into a scenario where JHtml can potentially resolve a helper class that was built for the service oriented approach and would therefore be unusable with the existing logic. The current logic mandates static helper functions. Using JHtmlJquery as an example if you remove the abstract keyword from the class and the static keyword from the framework method definition, then attempt to use $this, you will get a fatal error. Keeping the logic where a service must be explicitly registered to use the registry ahead of the path lookup logic, you avoid this scenario.

The intent with the service registry is to be able to make JHtml helpers that are non-static, not just move the classes to another spot in the filesystem. See my refactoring of the install app's helper class as an example. The registry will continue to support static method resolution and use, but that really isn't the preferred use of the API.

Either way there will be a hard B/C break when 5.0 comes around because of the deprecated API removal. That part cannot be mitigated without keeping the existing API in place, in which case I would just retract the service registry and keep the existing methodology and approach to JHtml in place forever.

[laoneo]

I understand your approach, but for the 3 series it is absolutely valid to introduce the registry and support already services. So it is legitime to do that in a BC way. In 4 we will be able to have the registry approach first and break BC in a minor way (if we want) and then removing it in 5. It is a much more hassle less way than just say with 5 we are going to remove it.

you get into a scenario where JHtml can potentially resolve a helper class that was built for the service oriented approach and would therefore be unusable with the existing logic

How can that happen, file lookups do only work for none namespaced files. This is the only way where automation is in place and thats only for the none namespaced code. Perhaps I do miss here something, can you give me an example how this situation can happen.

[mbabker]

Either a class is autoloaded (no path lookup) or a path lookup class gets resolved that is service designed, if for whatever reason that gets resolved it can and probably will result in errors. You might argue that it is more a developer error than something we need to care about, but if we're merging code to core knowing full well there is an easy code path that can result in generating errors we're doing something majorly wrong.

/**
 * This is the component.php file in our component's HTML helpers directory
 */
class JHtmlComponent
{
    public function thing(): string
    {
        return $this->doThing();
    }

    private function doThing(): string
    {
        return 'the thing';
    }
}

// This adds the class to the path lookups but the file isn't included yet
JHtml::addIncludePath(JPATH_COMPONENT . '/helpers/html');

// This part will immediately fatal out if you don't have the class loaded by now, but let's say for the sake of argument it is loaded, and since JHtml internally prefixes classes by default as JHtml...
JHtml::getServiceRegistry()->register('component', new JHtmlComponent);

// Now try to use our component helper
JHtml::_('component.thing');

With this PR as applied, the class will be resolved through the path lookups first and therefore attempt to use the class statically, uses of $this will cause errors. If the service registry logic remains in the way I introduced it, the path lookup logic never triggers and the service works correctly.

Because the use of the service registry changes the load order for things and potentially the internals of the helper class, the core libraries cannot be safely migrated except for with a major version bump because there is a high risk of B/C breakage. The service registry logic can be adapted into the HTML helper loader because it is new logic and 100% opt-in therefore cannot break anything with its introduction just by providing the API. Anyone migrating to it should at least document the change if they consider extension of JHtml helpers as part of their public API (and considering it's part of the JHtml design spec they probably should) and acting at a conservative level it would probably warrant a major version bump.

If we want to migrate all of core to service driven for 4.0, then fine. I think we can conservatively wait for 5.0 to do it, but you and I have butted heads since day one about how quickly code is shifted to new methodology and old methodology deprecated and removed. But it absolutely cannot happen in a non-major release for the core libraries. Migrating com_content or any extension level helpers, go for it. Migrating libraries/cms/html cannot be done.

[laoneo]

To all respect but this is a situation you describe is very unlikely to happen as it can only happen when somebody uses the new feature. It will be detected 100% during development of the component when somebody traps into that mistake. Instead of making a hard BC break on version 5, I suggest to make here a step by step transition.

But to move on and not argue this pr down, I suggest that I will revert the jquery service. In version 4 we can then again think about to ns the core services.

[mbabker]

You are not going to avoid a B/C break no matter how this code is implemented. The only way to not have a B/C break is to not remove the code I deprecated, to which point I would just revert my PR adding the registry and we be done with this. No matter how you try to spin my code under the false pretense of a backport, there are two pending B/C breaks:

1. Migration of a JHtml helper to the service registry. In doing so, any developer may also change the internals of their class (including refactoring from static methods to proper objects). The way I designed this, which you have broken by changing this to suit your desires, means that a service registry designed helper cannot be resolved and used with the existing filesystem path lookups. I have demonstrated EXACTLY why the registry feature MUST be implemented in the way that I have done so, and what the ramifications are if someone does something that results in their service class being resolved through the filesystem path lookup method incorrectly.

2. The removal of code which has been marked as deprecated. It doesn't matter that it's deprecated, the point in time in which it is removed creates a "hard B/C break".

@wilsonge make a call on this because I'm done here.

[laoneo]

I did not remove the code from you, and I reverted the JQuery service. So whats still wrong with it now?

[mbabker]

The two loading strategies are not compatible and you are attempting to force compatibility on it which actually causes the API to be more unpredictable.

A JHtml helper which contains only static methods MAY use the service registry and MAY use the path lookup logic.

A JHtml helper which contains non-static methods MUST use the service registry and MUST NOT use the path lookup logic.

If you are overloading a JHtml helper service through the JHtml::register() method, any callable (Closure, static method, class object) is acceptable.

If you are overloading a JHtml helper service through the path lookup logic, the resolved class MUST contain only static methods because that is all the path lookup logic knows how to support. Therefore, if I were to front load a lookup path to my custom JHtml class that replaces JHtmlBootstrap (this logic applies to any helper, do not focus on that single class name now), and JHtmlBootstrap is converted to a fully object oriented class at some point, as a developer I must be aware that my path lookup not only overrides the core class but also forces me to refactor the internals to be 100% static. I must also be aware that I must duplicate the entire class, whereas either replacing the service in the registry or using the existing JHtml::register() method allows me to subclass the service class and only replace the methods I desire.

The long and short of this is that in absolutely no way can the two loading strategies be made compatible because the service registry offers features that are not available through the path lookup logic. By making it possible for service registry classes to be overloaded by the path lookup logic, you remove the benefits of the service registry and override the fact that a developer opted for a B/C breaking API change by explicitly migrating from the path lookup method to service registry. This MUST be a B/C breaking migration simply because it will force services to require overloading in an alternate manner if you want to provide any form of assurance that the classes can be used as expected.

[laoneo]

So I also reverted the registry code. Now it is a simple namespace change this pr.