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

Fix deprecation of dynamic properties #16440

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Fix deprecation of dynamic properties #16440

merged 3 commits into from
Oct 2, 2023

Conversation

Jako
Copy link
Collaborator

@Jako Jako commented Jun 13, 2023

What does it do?

Define magic method __set and __get in the class to avoid the deprecation message. See: https://php.watch/versions/8.2/dynamic-properties-deprecated#__get-__set

There is also a fix included for an uncaught error: Cannot assign by reference to overloaded object in /core/src/Revolution/Hashing/modHashing.php:107.

Why is it needed?

Avoid the message Creation of dynamic property MODX\Revolution\Hashing\modHashing::$native is deprecated in core/src/Revolution/Hashing/modHashing.php on line 107

How to test

Those messages seem to occur during installing MODX 3.0.3 on PHP 8.2. See the screenshot from Slack:

image

There are other similar issues xPDO and modAccessibleObject that screenshot.

Related issue(s)/PR(s)

None known

Define magic method __set and __get in the class to avoid the deprecation message. See: https://php.watch/versions/8.2/dynamic-properties-deprecated#__get-__set
@Jako Jako requested review from opengeek and Mark-H as code owners June 13, 2023 14:12
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jun 13, 2023
@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (aed303b) 21.73% compared to head (9ca29e7) 21.73%.

Additional details and impacted files
@@            Coverage Diff            @@
##                3.x   #16440   +/-   ##
=========================================
  Coverage     21.73%   21.73%           
  Complexity    10482    10482           
=========================================
  Files           561      561           
  Lines         31612    31612           
=========================================
  Hits           6872     6872           
  Misses        24740    24740           
Impacted Files Coverage Δ
core/src/Revolution/Hashing/modHashing.php 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JoshuaLuckers
Copy link
Contributor

@Jako @opengeek

In PHP 9.0, dynamic properties will result in a fatal error.

@opengeek
Copy link
Member

@Jako @opengeek

In PHP 9.0, dynamic properties will result in a fatal error.

Not on classes that have that attribute they won't.

@rthrash rthrash added this to the v3.0.4 milestone Sep 21, 2023
@rthrash rthrash added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Sep 29, 2023
@opengeek opengeek merged commit 3a885e7 into modxcms:3.x Oct 2, 2023
opengeek added a commit that referenced this pull request Oct 2, 2023
* Fix deprecation of dynamic properties

Define magic method __set and __get in the class to avoid the deprecation message. See: https://php.watch/versions/8.2/dynamic-properties-deprecated#__get-__set

* Fix 'Cannot assign by reference to overloaded object'

* Use AllowDynamicProperties attribute instead of __get()/__set()

---------

Co-authored-by: Jason Coward <jason@opengeek.com>
@Jako Jako deleted the patch-1 branch February 7, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants