joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.76k stars 3.65k forks source link

How do we override classes now? #17992

Closed danyj closed 7 years ago

danyj commented 7 years ago

@mbabker

We used to override classes via JLoader::register within a plugin since 2.5 and worked without issues this is one example and we did override for 5 methods in 5 different classes

require_once($YjsgJLayoutFileDefaultFile);
JLoader::register('JLayoutFile', YJSGEXTEND . $IsJversion . '/layout/file.php', true);

the process was , copy exiting joomla class, rename it , load it before register , register exiting class with the extended method

3.8 is not lettings us do this anymore and half of the classes have been renamed

any suggestions ?

zero-24 commented 7 years ago

Would be great if you share your solution with the users that may found this issue using google thnaks 👍

danyj commented 7 years ago

@zero-24 Namespace was the issue , all classes have namespace now, if you do override remove the namespace

example https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Document/HtmlDocument.php#L9

danyj commented 7 years ago

il leave this open , @mbabker might have better suggestion.

mbabker commented 7 years ago

Unless I'm missing something the class aliasing was supposed to be done in a way where references to the old classes should've worked. @laoneo

danyj commented 7 years ago

@mbabker , thnx for reply ,

references to the old classes should've worked

does not seem like it , update broke all overrides , worked perfect in 3.7.5

laoneo commented 7 years ago

This should definitely work, there is even a unit test for it https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/joomla/JLoaderTest.php#L229. I will test your plugin on some point, how can I reproduce the issue? What for steps do I need to do after I have installed it?

danyj commented 7 years ago

@laoneo

how can I reproduce the issue

install free template and old plugin version on joomla 3.7.5

https://github.com/YJSGframework/eximium/releases/tag/1.0.1 https://github.com/YJSGframework/yjsg/releases/tag/2.2.8

update joomla to 3.8

you will see the fatal error.

the biggest issue was the classes move and a mix in all is that we never did an actual hard ( copy class and use our version ) class override with our version of a class

what we did instead in order to maintain Joomla changes is

get the class file and read it

https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/yjsg.php#L342 https://github.com/YJSGframework/yjsg/blob/master/yjsg.php#L440 https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/yjsg.php#L390

replace class name and save the new class in plugin

https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/yjsg.php#L391

https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/yjsg.php#L393

load the class and register override

https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/yjsg.php#L395-L397 https://github.com/YJSGframework/yjsg/blob/67b632a749b02f3747251ac19ffa24e50dc3bec0/includes/yjsgcore/classes/extend/30/module/helper.php

but since all class names have droped J and namespace is used , the replace could not find the class name to replace like JModuleHelper and the file was not there anymore

laoneo commented 7 years ago

I'v tested it with your extensions and the problem is that the the file which overrides the Joomla class is empty. I'm getting that error: Fatal error: Class 'YjsgJViewLegacyDefault' not found in /home/vagrant/Projects/joomla-cms/plugins/system/yjsg/includes/yjsgcore/classes/extend/30/component/view.php on line 21

So you are copying during runtime the class and do some search and replace on it? If you want to keep that functionality then yes you need to copy the new location. But honestly I don't suggest to use such a way you are doing.

brianteeman commented 7 years ago

I am closing this due to lack of response. It can always be reopened. But it doesnt look like a Joomla issue

danyj commented 7 years ago

@laoneo did not see the response till now. Sorry about that. the files move and aliasing messed up the process. Please note that this is 4th time that files move/class change since J3.x. Look at this https://github.com/YJSGframework/yjsg/blob/master/yjsg.php#L353-L392 , from version to version we must either find the file or lookup the new name ( caps changed or name ). It is a joke.

@brianteeman yes it is Joomla issue since we cant ever rely on bc and files/class names to be in the same place or have the name that they originally had. We fixed it but I am sure it will be same thing again in few months.

mbabker commented 7 years ago

The filesystem structure is not part of the API. And though the class names were changed, through class aliasing the old names still work AND you can still create a real class with the old non-namespaced name and overload the namespaced class when it is referenced through the alias. But class overloading should not be considered part of the API either and when you start doing that you really need to understand it's at your own risk.