Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ReLater opened this issue Jul 21, 2019 · 11 comments

Comments

@ReLater
Copy link
Contributor

commented Jul 21, 2019

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

#24858 (comment)

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.

@ReLater ReLater changed the title [4.0] onWebAssetBeforeAttach [4.0] onWebAssetBeforeAttach. 0 - Cannot set the argument result of the immutable event onWebAssetBeforeAttach Jul 21, 2019

@joeforjoomla

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

@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

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

Use it like this:

use Joomla\CMS\Event\WebAsset\WebAssetBeforeAttachEvent;

public function onWebAssetBeforeAttach(WebAssetBeforeAttachEvent $event)
{

}
@ReLater

This comment has been minimized.

Copy link
Contributor Author

commented Jul 21, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

@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

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

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

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

I hate reading about those kinds of borderline hacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.