[4.0] onWebAssetBeforeAttach. 0 - Cannot set the argument result of the immutable event onWebAssetBeforeAttach

ReLater commented 5 years ago

Steps to reproduce the issue

In a discussion about the B\C breaks as a result of implemented WebAssetManager thing in Joomla 4 and plugin events that can be used to get rid of assets that WebAsset thing imports in a B\C breaking way even if related JHtml helpers are overriden like it was possible in Joomla 3 over years

I found this comment of @Fedik

https://github.com/joomla/joomla-cms/pull/24858#issuecomment-491502468

such event already exist, called onWebAssetBeforeAttach https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/WebAsset/WebAssetManager.php#L343

Thus I added
```
public function onWebAssetBeforeAttach($a, $b)
{
}
```
in a system plugin, e.g. the sef plugin, and tried several variations of that code

Expected result

At least: Nothing happens.

Actual result

SEF plugin, Backend and frontend (ignores debug mode (no call stack)).

Error: Cannot set the argument result of the immutable event onWebAssetBeforeAttach.: Cannot set the argument result of the immutable event onWebAssetBeforeAttach.

In a more complex custom plugin I get a call stack

Call stack
--
# | Function | Location
1 | () | JROOT\libraries\src\Event\AbstractImmutableEvent.php:67
2 | Joomla\CMS\Event\AbstractImmutableEvent->offsetSet() | JROOT\libraries\src\Plugin\CMSPlugin.php:289
3 | Joomla\CMS\Plugin\CMSPlugin->Joomla\CMS\Plugin\{closure}() | JROOT\libraries\vendor\joomla\event\src\Dispatcher.php:495
4 | Joomla\Event\Dispatcher->dispatch() | JROOT\libraries\src\WebAsset\WebAssetManager.php:349
5 | Joomla\CMS\WebAsset\WebAssetManager->attachActiveAssetsToDocument() | JROOT\libraries\src\Document\Renderer\Html\MetasRenderer.php:54
6 | Joomla\CMS\Document\Renderer\Html\MetasRenderer->render() | JROOT\libraries\src\Document\Renderer\Html\HeadRenderer.php:36
7 | Joomla\CMS\Document\Renderer\Html\HeadRenderer->render() | JROOT\libraries\src\Document\HtmlDocument.php:499
8 | Joomla\CMS\Document\HtmlDocument->getBuffer() | JROOT\libraries\src\Document\HtmlDocument.php:798
9 | Joomla\CMS\Document\HtmlDocument->_renderTemplate() | JROOT\libraries\src\Document\HtmlDocument.php:570
10 | Joomla\CMS\Document\HtmlDocument->render() | JROOT\libraries\src\Application\CMSApplication.php:970
11 | Joomla\CMS\Application\CMSApplication->render() | JROOT\libraries\src\Application\SiteApplication.php:744
12 | Joomla\CMS\Application\SiteApplication->render() | JROOT\libraries\src\Application\CMSApplication.php:247
13 | Joomla\CMS\Application\CMSApplication->execute() | JROOT\includes\app.php:63
14 | require_once() | JROOT\index.php:36

System information (as much as possible)

J!4 nightly of today, XAMPP PHP 7.2 and PHP 7.3/Linux (at a host)

Additional comments

After several days of revision of J3 extensions to make them J4 ready I'm frustrated. From my point of view codes that are marked as "deprecated" should work as with Joomla 3 until they are removed from core completely in the end. The WebAssetManager is an example that ignores that completely and needs nasty/additional hacks.

joeforjoomla commented 5 years ago

@ReLater the onWebAssetBeforeAttach event works, however due to latest change contrary to what i said it was right, you should use the onBeforeCompileHead to remove scripts added by onWebAssetBeforeAttach that is fired just before.

However as @mbabker discussed deeply, this WebAssetManager thing in Joomla 4 should not be there at all and i hope that the advice of @mbabker will be followed completely removing it.

SharkyKZ commented 5 years ago

Use it like this:

use Joomla\CMS\Event\WebAsset\WebAssetBeforeAttachEvent;

public function onWebAssetBeforeAttach(WebAssetBeforeAttachEvent $event)
{

}

ReLater commented 5 years ago

Use it like this:

Nice idea that works without errors but the $event or "arguments" or "subject" or whatever are not debuggable even wirh 5GB memory. Or foreach=>(to empty file even if I just want the keys of this or that). Thus it's still just a mystery in the end.

you should use the onBeforeCompileHead to remove scripts added by onWebAssetBeforeAttach that is fired just before.

That doesn't work for me. I use a plugin that "overregisters" HTMLHelpers (also) like 'jquery.framework' as early as possible e.g. to load jquery from CDN (for users configurable in a SIMPLE json file with other helpful and easy to manage things). Additionally it blocks some core HTMLHelper methods by setting static::$loaded to true to get rid of possible static::framework() calls inside the core JHtml class. Easy thing.

static::$loaded switch has been removed also from some of the methods, whyever.

If now a plugin comes around the corner like the debug plugin that fires

$assetManager->enableAsset('jquery-noconflict');

that loads Jquery because of the according dependency in joomla.asset.json via WebAssetManger before all other scripts. JQuery is loaded twice and others of my JQuery dependent scripts are loaded before "my" JQuery is loaded.

In theory one could reorder all that sh.. in onBeforeCompileHead but it leads the whole configurable framework plugin ad absurdum because I need a second plugin (run as last one) and have to add custom registries and so on.

And no, adding Jquery and other scripts etc. as overrides in any template that uses the plugin is not an option.

However as @mbabker discussed deeply, this WebAssetManager thing in Joomla 4 should not be there at all and i hope that the advice of @mbabker will be followed completely removing it.

I have read all threads and comments concerning WebAssetManager and I don't believe in it ;-) I only see the immense problems we have here since days. The whole system is too complicated for us and the json files just a hell for us/users (if we would understand them ;-) )

Fedik commented 5 years ago

I use a plugin that "overregisters" HTMLHelpers (also) like 'jquery.framework' as early as possible e.g. to load jquery from CDN

Adding asset with the same name will override previous asset, in the asset registry.

$wa = $doc->getWebAssetManager();
$wr = $wa->getRegistry();
$wr->add(new WebAssetItem('jquery', ['js' => ['http://foobar.com/jquery.js']]));

this will overridden jQuery, can be called at onWebAssetBeforeAttach, beforeRender, afterDispatch

Other way, add your joomla.asset.json, at same event, that contain overrides:

$wa = $doc->getWebAssetManager();
$wr = $wa->getRegistry();
$wr->addRegistryFile('media/plg_system_foobar/joomla.asset.json');

with:

{
  "name": "plg_system_foobar",
  "version": "4.0.0",
  "description": "Joomla CMS",
  "license": "GPL-2.0+",
  "assets": [
    {
      "name": "jquery",
      "js": [
        "http://foobar.com/jquery.js"
      ]
    },
    {
      "name": "bootstrap.css",
      "css": [
         "http://foobar.com/bootstrap.js"
      ]
    }
  ]
}

which override jquery and bootstrap (css), if any is in use

joeforjoomla commented 5 years ago

@Fedik it would be useful to have a developer documentation showing the sequence of events as fired by the core and an explanation of what to do and what to not do in each of them. For example:

... onAfterRoute -> do not instantiate document onAfterDispatch -> ... onWebAssetBeforeAttach -> ... onBeforeCompileHead -> do not use WebAssetsManager that is already locked ...

joeforjoomla commented 5 years ago

I have read all threads and comments concerning WebAssetManager and I don't believe in it ;-) I only see the immense problems we have here since days. The whole system is too complicated for us and the json files just a hell for us/users (if we would understand them ;-) )

@ReLater i agree with you and @mbabker , the WebAssetsManager definitely is a superstructure to the Document one and probably brings more issues than benefits.

mbabker commented 5 years ago

The whole system is too complicated for us

That's why I pointed out in my other item that introducing the asset manager requires a major paradigm shift and it's not something that can be eased into like was attempted with the current code, you basically have to go all in on it or decide to stick with the 3.x status quo. And it does have its benefits over the data model and API that exists now (also pointed out elsewhere, so not going to rehash) so it's not like it's adding complexity for the sake of complexity, but at the end of the day it's a major change and one that needs to be planned out very meticulously for it to be successful.

ReLater commented 5 years ago

Adding asset with the same name will override previous asset

I know. The magic word here is previous and that's why I have written

In theory one could reorder all that sh.. in onBeforeCompileHead but it leads the whole configurable framework plugin ad absurdum because I need a second plugin (run as last one) and have to add custom registries and so on.

As long as we have a mixture of possibilities and no "absolute switches like static::$loaded" it's not worth the trouble to continue the development.

I'll close here in some days. The issue contains some helpful informations that perhaps are of interest to the one or another.

Fedik commented 5 years ago

That another issue about plugin order, You may have onBeforeCompileHead and use "nice old static::$loaded", but no one guarantee that your plugin always will be "last one". There always possible a condition that someone run another plugin after your.

At current point of time (because future may be different): for an asset override a last possible event is onWebAssetBeforeAttach.

ReLater commented 5 years ago

I have a order() method in my plugin listening on some events in core. Thus it's always guaranteed that it's the first (or last).

mbabker commented 5 years ago

I hate reading about those kinds of borderline hacks.

joomla / joomla-cms