The JPATH_COMPONENT
path constants are really not constant at all; they map to the active component as defined by the request and any use of them in extensions is inconsistent and potentially breaking (i.e. if you tried using a model from another component that is including a file with that constant, if it's not something "standard" you're going to get a broken page).
I propose these be deprecated and the core code refactored to not make use of them (retaining them in 4.x since IMO this might be too big of a change to introduce as a deprecation in 3.7 and removed in 4.0).
Labels |
Added:
?
|
Ps all for refactoring to not use them in core
Yes, 5.0 would be when the constants are removed completely.
Hmm so we need to hard code any component folder name in the component?
JPATH_ADMINISTRATOR . '/components/com_example/'
Or do i miss something?
Yes, that would be the preferred convention.
As stated, the problem is JPATH_COMPONENT
(and its children) is not constant. When I'm on a com_content view in the frontend, it's path is JPATH_ROOT . '/components/com_content
and on com_modules backend it's JPATH_ROOT . '/administrator/components/com_modules'
.
Take for example the frontend BannersModelBanner
class. It has this line at the beginning:
JTable::addIncludePath(JPATH_COMPONENT_ADMINISTRATOR . '/tables');
If you're loading that model while the request was for a com_content view, that line is going to add JPATH_ROOT . '/administrator/components/com_content/tables
to the lookup path in JTable
, which obviously means that the BannersTable*
classes won't be included in the path lookup and any operation needing those will fail.
Right now it's less of an issue in controller or view classes because the chainability or or reusability of those is non-existent in our app structure. But in models which are better structured for reusability or other extension types, this could really cause unintended or completely broken behaviors.
Pretty sure I came up against this today with the image manager
Not to mention our path (un-)constant uses in general sometimes causes testability issues with the app. Take for example JApplicationAdministratorTest::testGetTemplate().
We have some constants that are relative to the active app (so JPATH_BASE
, JPATH_CACHE
, and JPATH_THEMES
will be different if you're on the frontend or in the admin section). Because we have to define all the path constants in the unit test bootstrap to ensure things work right, and because constants can only be defined once in a process, this test can never run correctly because the function assumes that const JPATH_THEMES = JPATH_ADMINISTRATOR . '/templates'
while in the test environment const JPATH_THEMES = JPATH_SITE . '/templates'
.
What about a JPATH_COMPONENTS_ADMIN = JPATH_ADMINISTRATOR . 'components';
JPATH_COMPONENTS_SITE = JPATH_ROOT . 'components'; so we reduce the hard coded part to just the component name?
It's still "hardcoded". The difference is where that "hardcoded" segment is defined. Unless you're saying we're going to change the path structure at some point and no longer have administrator/components
or components
directories or make those paths configurable, I don't see the benefit in adding more path constants like that.
Ok just some random ideas :)
Actually, without refactoring a lot of internals, Joomla would break massively if you tried to make component paths customizable.
For example, in the extension installer library:
$this->parent->setPath('extension_site', JPath::clean(JPATH_SITE . '/components/' . $this->element));
We're nowhere near being in a state to support moving around most of the filesystem. Let's not make things more complex
No let's do that and move everythingt. We can then write scripts to
automatically move everything in 3pd extensions. It will be easy and fully
B/C. Oh wait that's joomlaX
Status | New | ⇒ | Discussion |
Why you're even discussing hardcoding folders in the code?
I've thought that there was a statement to leap for PSR-4 autoloading in 4.0?
Certain base paths are always going to have to be "hardcoded" (i.e. JPATH_ADMINISTRATOR
, JPATH_PLUGINS
, etc.) to know where to find things. Hardcoded is used loosely because you basically have to have a known path to find this stuff defined somewhere. There's no point in us trying to find plugin classes in components/com_content/helpers
.
Likewise, all non-PHP files can't be hooked into an autoloader (so the XML definitions used within JForm
being the prominent example, the other is the XML extension manifests).
Lastly, though autoloading is a goal, right now nobody is actively working on it outside of the libraries and we're still probably going to carry forward most of the JPATH_
constants even when we have it. What I want to do though is get rid of the unconstant constants, and these three (JPATH_COMPONENT
, JPATH_COMPONENT_SITE
, and JPATH_COMPONENT_ADMINISTRATOR
) are the biggest offenders and most problematic to our code base.
I believe "hardcoded" should take place only in autoloader. Exceptions from the rule are allowed, but no need in encouraging developers for using them.
What for was the "plugin" part in your comment?
$var = new Akeeba\Plugin\System\Akeebaupdatecheck();
$var = new Core\Plugin\System\Log\Helper();
$var = new Core\Plugin\System\Log();
Simple, isn't it? Call it from plugins, from components, no matter where...
Well, if they can't be, they almost can be, or even can be, but need to put some work into.
For now I didn't figure out autoloading of file extensions.
But I believe it will be very soon. You can look into JooYii to see how I've managed views, fileds, layouts autoloading(preserving joomla overrides), or finding minifest.xml path at JooGii (codegenerator).
You will always need to force people to learn something new and do something better, even if this will help them to make better products.
So leave constants - good result if atleast 1% of developers will atleast try autoloading.
Well I don't want to argue as I've learnt already it's deadend anyway...
It just was wierd to see this approach still will take place after PSR-4 autoloader was proposed
We're mixing things here honestly. PSR-4 autoloading isn't dependent on support (or removing support) for the JPATH_
constants. It's also only a partial solution since we have so many entities that don't fall into place under a PSR-4 convention. These are solutions that are better left using manual path lookups, so you still need something to define and build some standardized paths.
This is a step in the right direction. We ultimately end up with less "hardcoded" path constants, especially those of an unconstant nature.
It's really can be avoided just need to work a little bit on it.
As I've mentioned there is already fruits of labor on working on autoloading everything and using more standard MVC approach than Legacy. Also there is way to avoid unnecessary form.xml files and sql queries and let tables auto-populate forms and fix themselves on fly.
Category | ⇒ | Code style |
I don't use these constants anymore in my extensions, because of the issue that they change. It would be nice to deprecate them in 4 and remove them in 5. I think it's doable to remove the 178 usages in core
Labels |
Added:
J4 Issue
|
Status | Discussion | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-08-18 07:30:38 |
Closed_By | ⇒ | joomla-cms-bot |
Closed_Date | 2018-08-18 07:30:38 | ⇒ | 2018-08-18 07:30:39 |
Closed_By | joomla-cms-bot | ⇒ | franz-wohlkoenig |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/12415
If this isn't removed in 4.0 then when would it be removed? In 5.0?
On 14 Oct 2016 3:32 p.m., "Michael Babker" notifications@github.com wrote: