[5.0][Events] Fix backward compatibility for onAfterRenderModules

[Fedik]

Summary of Changes

Fix backward compatibility for onAfterRenderModules

Testing Instructions

Add following listener to Sef system plugin:

public function onAfterRenderModules(&$value) {
      $value .= ' aaaa';
}

Open the site, and check rendered modules

Actual result BEFORE applying this Pull Request

Nothing changed

Expected result AFTER applying this Pull Request

Every module position now have 'aaaa' appended

Link to documentations

Please select:

• Documentation link for docs.joomla.org:
• No documentation changes for docs.joomla.org needed
• Pull Request link for manual.joomla.org: New Event classes Manual#177
• No documentation changes for manual.joomla.org needed

Reference:

• [5.0][Events] Module event classes #41413

[HLeithner]

Merging this as with code review and based on the other similar PRs, before beta1 to get enough feedback for final release.

Additional ArrayProxy is nothing we plan to have for ever right? because it only hides the & reference, Also I think we should convert the array back after we triggered the event with them?

[Fedik]

Additional ArrayProxy is nothing we plan to have for ever right? because it only hides the & reference

Yeah, it for replacement of array with reference.
There many events that use "array" editing.
Tricky thing.

Also I think we should convert the array back after we triggered the event with them?

The changes applied to original array, converting it back is unnecessary.

[HLeithner]

Sure but the thing is we don't want that the Plugin manipulate the array directly?

[Fedik]

Yes, that the idea. because it hard to debug where the data gets modified by reference.
With ArrayProxy it at least possible to attach debugger to ArrayProxy::offsetSet().
We have around 15-20 events using array modification.

Unfortunately, there also events where we mix object/array for $data,

[HLeithner]

we should make this consistent... maybe not in 5 but later

[Fedik]

TBH, I have no idea how it can be done, for now and in future.

[Fedik]

Maybe in one of next beta, I will add updateData() method, to all events with $data.
This will enforce developers to use $event->updateData($data);, instead of editing $data "in place", to prepare them for future.

Similar what we made for strings $downloadEvent->updateUrl($url);

Need to think.

[HLeithner]

We have $event->setData() don't we have this to update arguments? (when not immutable?)

[Fedik]

$event->setData()

It is part of argument validation, it cannot be public, or you get a loop. (Do not ask why)

joomla-cms/libraries/src/Event/AbstractEvent.php

Lines 202 to 208 in a78c8f5

	$methodName = 'set' . ucfirst($name);
	if (method_exists($this, $methodName)) {
	$value = $this->{$methodName}($value);
	}
	return parent::setArgument($name, $value);