PHP 8.2 - Deprecate dynamic properties

[icyz]

hello everyone,
is it planned to face the problem of "Creation of dynamic property"? I don't know, but this will be a bit painful.

Deprecated: Creation of dynamic property Zend_View::$var is deprecated

[Hikariii]

I think this is impossible to fix. Zend_View created properties dynamically. This looks like zend_view is not usable like this in php8.2

[glensc]

you can still use the attribute:

• https://php.watch/versions/8.2/readonly-classes#dynamic-properties
• https://php.watch/versions/8.2/AllowDynamicProperties
• https://php.watch/versions/8.2/dynamic-properties-deprecated

[Hikariii]

The #[AllowDynamicProperties] attribute would be a nice solution here indeed.

[Hikariii]

This would also be a solution for https://github.com/Shardj/zf1-future/blob/release-1.21.4/library/Zend/Form/Element.php#L863-L878

[glensc]

The setAttrib could be rewritten to store data in $this->data container, but it requires the properties are not actually accessed.

[icyz]

you can still use the attribute:

• https://php.watch/versions/8.2/readonly-classes#dynamic-properties
• https://php.watch/versions/8.2/AllowDynamicProperties
• https://php.watch/versions/8.2/dynamic-properties-deprecated

Would this be a temporary solution? or permanent?
I think the #[AllowDynamicProperties] attribute won't fix the problem, just postpone it...

[Hikariii]

Reading up on https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.dynamic-properties it looks like implementing a __set() and __get() method is more future-proof.

[Hikariii]

On the other hand, not using this repository is even way more future-proof

[icyz]

On the other hand, not using this repository is even way more future-proof

but we love this repository, so we have to do the best we can.

[glensc]

You can also extend stdClass which is written on those links that it already has #[AllowDynamicProperties] set automatically. I don't think either of them is going away. not the attribute nor stdClass. I prefer #[AllowDynamicProperties] because it's purpose is distinct, extending stdCass may introduce some other unwanted funny bugs.

[Hikariii]

I see #[AllowDynamicProperties] has already been used. Hikariii@f6f029c
As I see it this changes the composer requirement to "php": ">=8.0" because php7 does not know about attributes. So it looks like an inconsistency has gotten into the codebase, or the composer requirements are not updated.

[glensc]

no. #[AllowDynamicProperties] doesn't change requirements, because php7 doesn't understand attributes neither does it complain about dynamic properties.

[Hikariii]

ok that's good then adding the attribute should be the way to go

[vijaygarg-ahead]

Can anyone give a permanent solution to this issue?

[VentyCZ]

This should have been fixed in 1.22.0 if I'm not mistaken. Zend_View_Abstract contains changes needed, see #261.

[thijsvdanker]

The fixes in #261, specifically commit 34331f7, breaks our application.

We have wrapped Laravel around our ZF application and run the Blade compiler after ZF has done it's job.

If #[AllowDynamicProperties] keeps the code before 34331f7 running that would be great for us 😬

[glensc]

@thijsvdanker you need to provide a standalone reproducer for your report to be useful.

[thijsvdanker]

@glensc I understand.
I don't really think my usecase is technically sound enough to be part of the discussion. I just wanted to put forward that if all else equal, the "non-breaking" change would have my preference.

[nunnrlc]

The fixes in #261, specifically commit 34331f7, breaks our application.

We have wrapped Laravel around our ZF application and run the Blade compiler after ZF has done it's job.

If #[AllowDynamicProperties] keeps the code before 34331f7 running that would be great for us 😬

Same here, broken after putting the view dynamic properties into private $_data = []; in library/Zend/View/Abstract.php
Reason is use of property_exists() in our codebase. Luckily only once, and caught in testing, so not a big deal here!

Actually, I don't mind the zf1-future change, but just to help others I'll explain the fix...

In our codebase we had property_exists() to test for a view variable in a phtml template

if (property_exists($this, 'someViewProperty')) { // $this is the view instance.
  ...
}

Well that doesn't work now the dynamic properties are stored in a __get/__set array after commit 34331f7

Easily changed our code to fix with isset():

if (isset($this->someViewProperty)) { // $this is the view instance.
  ...
}

So maybe a mistake to ever use property_exists rather than isset. The isset function plays nicely with __get/__set, but property_exists is strictly honest about "real" properties. A PHP gotcha I guess.

[holtkamp]

When using the ContextSwitch ActionHelper, a 'contextKey' is used which defaults to contexts:

zf1-future/library/Zend/Controller/Action/Helper/ContextSwitch.php

Line 62 in 7fa9c5f

protected $_contextKey = 'contexts';

No setters are available to influence the value of the 'contextKey'.

In various parts of the class it then checks whether this property is set on the Controller and if not, will set it:

zf1-future/library/Zend/Controller/Action/Helper/ContextSwitch.php

Lines 1145 to 1149 in 7fa9c5f

	$contextKey = $this->_contextKey;
	if (!isset($controller->$contextKey)) {
	$controller->$contextKey = [];
	}

I circumvented / resolved this specific deprecation by adding public array $contexts property in my own base Controller with a comment about why that property exists.

But as suggested in #307 (comment) #[AllowDynamicProperties] could be added to https://github.com/Shardj/zf1-future/blob/7fa9c5f50f0ce8ceecafb13e107eebe4c3d04051/library/Zend/Controller/Action.php

[develart-projects]

Feel free to create an PR for that and add #[AllowDynamicProperties]. It makes no sense to rework it.

[hungtrinh]

This issue resolved here: 34331f7 by PR #268

[develart-projects]

@hungtrinh well, using private properties should be forbidden, unless properly justified. Completely breaking possible inheritance. Anyway, thanks for pointing that out, we are done here then :)