User tests: Successful: Unsuccessful:
Pull Request for Issue #34228 .
With this option extension developers can decide on their own use case if they want to use full js / css files or the minified versions without being forced to switch the entire site into debug mode (global option)
add a script to your extensions webpage
HTMLHelper::_('script', 'media/system/js/showon.js', ['debugMode' => true]);
minified version is loaded
<script src="/media/system/js/showon.min.js"></script>
full version is loaded
<script src="/media/system/js/showon.js"></script>
? don't think there is documentation on this specific method
| Status | New | ⇒ | Pending |
| Category | ⇒ | Libraries |
| Title |
|
||||||
| Title |
|
||||||
I not sure here.
It will be confusing to have 2 option$detectDebugand$debugMode
detectDebug is for me the confusing one: it doesn't detect debug but what it does is when set force minified versions even when debug is enabled.
debugMode is exactly what it is: a toggle set to debug ( = full version of the scripts / css) even when JDEBUG is set to no.
$debugMode is already used in the function, what I do in this PR is make it an option to the function instead of set it in the function to default value false.
As an extension developer when debugging I want to debug as close to 'production' as possible. Using JDEBUG changes not only the behavior of my extension but of all extensions / template / etc. and Joomla.
This option allows me to only change my extension for debugging where the rest remains as it normally is.
detectDebug is for me the confusing one: it doesn't detect debug but what it does is when set force minified versions even when debug is enabled.
Maybe better if we can fix this part somehow, to keep original file name. Instead of introducing another option, or?
Using JDEBUG changes not only the behavior of my extension but of all extensions / template / etc. and Joomla.
yeah, true
Maybe better if we can fix this part somehow, to keep original file name. Instead of introducing another option, or?
For me that would be a B/C break. I learned these variables by debugging through the code as these are nowhere (nowhere I could find them) described, but it is possible that these are actively used by other devs?
adding $debugMode as variable (this PR) is fully B/C and gives control over all possible combinations
Hi, it has been months already. Can somebody please advice or make a decision?
Unfortunately it will not go to J4 as it is, because signature change of includeRelativeFiles().
Hm, need to think something else.
Unfortunately it will not go to J4 as it is, because signature change of
includeRelativeFiles().
J4 as in 4.0.x or J4.x?
for 4.0.x for sure,
hm, maybe for J4.x, but still risky to me :)
Thanks, but who decides on this? Is this somewhere on the agenda to be looked at?
Spent a lot of time on this to troubleshoot and come up with an (non B/C breaking) improvement.
Cannot say,
I guess first it need to be tested ;)
It needs at least 2 successfull tests, then it will be marked as Ready To Commit, and after is up to Release Leader to decide upon
It needs at least 2 successfull tests, then it will be marked as Ready To Commit, and after is up to Release Leader to decide upon
Changes like these will never make it into core as this is a pretty specific (developer) use case that other developers have probably worked around for a long time (as I did).
furthermore: this is really a nobrainer and the impact can be determined by a simple code review: you can see that it doesn't break B/C.
For other changes I did I spent a lot of time rallying for testers amongst my clients, this will not work for these kind of changes....
Fingers crossed that another dev will run into this and takes the time to look for a solution other then to build a work around.
just my 2cts.
Will keep this PR open for a while, see what happens.
Not a big fan of long time open PR's as every time I see them I wonder 'why did I bother' :) not so motivating #lol
you can see that it doesn't break B/C.
Unfortunately it is, changing method signature lead to fatal error on PHP 8 https://php.watch/versions/8.0/lsp-errors ,
and I am sure there is a people who extends HTMLHelper
Will keep this PR open for a while, see what happens.
yes please
Unfortunately it is, changing method signature lead to fatal error on PHP 8 https://php.watch/versions/8.0/lsp-errors ,
and I am sure there is a people who extends HTMLHelper
when extending HtmlHelper and in that extended class explicitly extending the protected includeRelativeFiles, php 7 will give a warning, php 8 an error.
But IMO that is part of maintaining your own code, that should not stop progress on the Joomla side because if it did, Joomla would still be on php5 probably :)
On quick test it works good.
Please also update this part
joomla-cms/libraries/src/WebAsset/WebAssetItem.php
Lines 312 to 320 in b4fc838
To use this option:
// Get the file path
$file = HTMLHelper::_(
$type,
$path,
[
'pathOnly' => true,
'relative' => !$this->isPathAbsolute($path),
'debugMode' => $this->getOption('debug', false),
]
);| Labels |
Added:
?
|
||
This pull request has automatically rebased to 4.2-dev.
This pull requests has been automatically converted to the PSR-12 coding standard.
| Title |
|
||||||
| Labels |
Added:
PR-4.3-dev
?
Removed: ? |
||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-10-23 08:32:55 |
| Closed_By | ⇒ | bembelimen |
Hello @Ruud68 we like this idea, but as we have the asset manager now, we should implement this e.g. in the joomla.asset.json to set debug for certain files individually not using the HTMLHelper anymore. Additionally it is a B/C break when we merge it into 4.3.
Probably you're willing to give it a try directly in the Webasset Manager for Joomla! 5?
Hi @bembelimen thanks for taking the time to review and appreciate this contribution. I appreciate it!
I closed this, and other PR's I did for Joomla, a long time ago in my mind. Lessons learned :)
I not sure here.
It will be confusing to have 2 option
$detectDebugand$debugMode