Skip to content

[1.x] Dynamic properties handling#563

Merged
nunomaduro merged 1 commit intopestphp:1.xfrom
fabio-ivona:1.x-dynamic-properties
Aug 25, 2022
Merged

[1.x] Dynamic properties handling#563
nunomaduro merged 1 commit intopestphp:1.xfrom
fabio-ivona:1.x-dynamic-properties

Conversation

@fabio-ivona
Copy link
Collaborator

@fabio-ivona fabio-ivona commented Aug 25, 2022

Implements dynamic properties handling in order to avoid deprecation in PHP 8.2

fix #555

a PR for v2 will follow this one

@fabio-ivona fabio-ivona requested a review from nunomaduro August 25, 2022 11:14
@nunomaduro nunomaduro changed the title dynamic properties handling [1.x] Dynamic properties handling Aug 25, 2022
@nunomaduro
Copy link
Member

Can't you solve this issue using the #[AllowDynamicProperties] thing?

@fabio-ivona
Copy link
Collaborator Author

Can't you solve this issue using the #[AllowDynamicProperties] thing?

just wanted to fix the issue in a definitive way, but I can do that, np

@fabio-ivona
Copy link
Collaborator Author

@nunomaduro done

@nunomaduro nunomaduro merged commit 8e695d6 into pestphp:1.x Aug 25, 2022
@nunomaduro
Copy link
Member

Can you port that to pest v2?

@dingo-d
Copy link
Collaborator

dingo-d commented Aug 29, 2022

@fabio-ivona @nunomaduro this is not really a fix 😬

You are just silencing the deprecation until PHP 9 🙈

The underlying test case should probably have a __set($property, $propertyValue) method or something similar in order for the properties to be creatable on the fly.

That way, any reference to the property inside the test case that doesn't exist on the TestCase class would be created on the fly.

EDIT: Just noticed this: #561 and it seems to be the good direction for handling this.

@fabio-ivona
Copy link
Collaborator Author

fabio-ivona commented Aug 29, 2022

As it is a deprecation, it is fine to silence it, we'll plan a clean fix for php9

Later we implement a fix, the more info we'll have about future development/fixes on pest core. This will allow to better integrate the chosen solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants