Skip to content

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

Except for final and test classes, we have to account for inheritance and for existing payloads in both directions (old apps accepting payloads from updated apps and updated apps accepting them from old apps.)

This means the fix is to mimic what the engine does currently, with all the existing quirks regarding private properties.

Don't merge unless php/php-src#19435 is.

@carsonbot carsonbot added this to the 6.4 milestone Aug 13, 2025
@carsonbot carsonbot changed the title Php85 sleep Php85 sleep Aug 13, 2025
@nicolas-grekas nicolas-grekas changed the title Php85 sleep Account for PHP 8.5 deprecating __sleep()/__wakeup() Aug 13, 2025
@nicolas-grekas nicolas-grekas force-pushed the php85-sleep branch 2 times, most recently from bd46163 to 0ce7a72 Compare August 13, 2025 15:04
public function __sleep(): array
{
return ['string'];
}

public function __wakeup(): void
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to add the method so that __unserialize can call it, in case a child class defines it.
We could do a method_exists check, but that's be adding more boilerplate.

Comment on lines 80 to 111
/**
* @internal
*/
public function __serialize(): array
{
$data = [];
foreach ($this->__sleep() as $key) {
$data[$key] = $this->$key;
}

return $data;
}

/**
* @internal
*/
public function __unserialize(array $data): void
{
foreach ($data as $key => $value) {
$this->$key = $value;
}

$this->__wakeup();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might these make more sense as a trait or two, for easier reuse?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this concern is cross-components - we could have one trait per component, but that'd be just garbage code...

@nicolas-grekas
Copy link
Member Author

My patch doesn't work BTW, I forgot to account for mangled property names.

@nicolas-grekas
Copy link
Member Author

My patch doesn't work BTW, I forgot to account for mangled property names.

fixed, more garbage code...

@nicolas-grekas
Copy link
Member Author

fixed, more garbage code...

well, not so so
the updated code breaks child classes that list private properties.

This deprecation is a net loss.

@nicolas-grekas nicolas-grekas deleted the php85-sleep branch August 13, 2025 15:36
@nicolas-grekas nicolas-grekas restored the php85-sleep branch August 13, 2025 15:51
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Aug 13, 2025

the updated code breaks child classes that list private properties.

I found the way to fix this: closure rebinding + reflection. Not pretty at all.

@nicolas-grekas nicolas-grekas force-pushed the php85-sleep branch 3 times, most recently from 579912b to 30d0ed7 Compare August 13, 2025 16:30
nicolas-grekas added a commit that referenced this pull request Aug 14, 2025
…n't a concern (nicolas-grekas)

This PR was merged into the 7.4 branch.

Discussion
----------

 Replace __sleep/wakeup() by __(un)serialize() when BC isn't a concern

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Subset of #61407

* Final classes can be moved to `__(un)serialize()` - payload compatibility is preserved
* Throwing sleep/wakeup - even-though those could be considered BC breaks, I don't expect anyone overwrote them to make the corresponding classes serializable

Commits
-------

246daf3 Replace __sleep/wakeup() by __(un)serialize() when BC isn't a concern
nicolas-grekas added a commit that referenced this pull request Aug 14, 2025
…nd data collectors and make `Profile` final (nicolas-grekas)

This PR was merged into the 7.4 branch.

Discussion
----------

[HttpKernel] Deprecate `__sleep/wakeup()` on kernels and data collectors and make `Profile` final

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | yes
| Issues        | -
| License       | MIT

Related to #61407

FTR, I searched for code extending `Profile` on github and found none.

Commits
-------

c54120e [HttpKernel] Deprecate `__sleep/wakeup()` on kernels and data collectors and make `Profile` final
nicolas-grekas added a commit that referenced this pull request Aug 14, 2025
…ace `__sleep/wakeup()` by `__(un)serialize()` (nicolas-grekas)

This PR was merged into the 8.0 branch.

Discussion
----------

[HttpKernel][Mime][Serializer][String][Validator] Replace `__sleep/wakeup()` by `__(un)serialize()`

| Q             | A
| ------------- | ---
| Branch?       | 8.0
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | Fix #61407
| License       | MIT

This gets rid of all usages of `__sleep/wakeup()` while preserving FC/BC for payloads.

Commits
-------

2b841c1 [HttpKernel][Mime][Serializer][String][Validator] Replace `__sleep/wakeup()` by `__(un)serialize()`
@nicolas-grekas
Copy link
Member Author

Symfony 8 won't use sleep/wakeup anymore, see #61424
This required adding deprecations to branch 7.4 so that child classes could know about the change.
The removed lines in #61424 are this deprecation layer. 🤮

@nicolas-grekas nicolas-grekas deleted the php85-sleep branch August 14, 2025 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants