Skip to content

[VarExporter] Fix lazy objects with hooked properties #59761

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

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 12, 2025

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #59153
License MIT

With hook proxies when needed so that lazy objects are fully compatible with non-final hooks.

@carsonbot carsonbot added this to the 6.4 milestone Feb 12, 2025
@nicolas-grekas nicolas-grekas changed the title [VarExporter] Fix ignoring hooked properties [VarExporter] Fix lazy objects with hooked properties Feb 13, 2025
@nicolas-grekas nicolas-grekas merged commit 88649d9 into symfony:6.4 Feb 13, 2025
2 of 10 checks passed
@stof
Copy link
Member

stof commented Feb 13, 2025

I think this should have more tests to cover more cases:

  • virtual properties with only a get hook
  • virtual properties with both a get and a set hook (this is currently covered)
  • backed properties with a get hook and no set hook (i.e. writing directly to the backing storage)
  • backed properties with a set hook and no get hook (i.e. reading directly from the backing storage)
  • backed properties with both hooks (this is currently covered)

@nicolas-grekas nicolas-grekas deleted the ve-hooks branch February 13, 2025 11:38
@nicolas-grekas
Copy link
Member Author

That'd be nice indeed, help wanted!

@Rixafy
Copy link

Rixafy commented Feb 22, 2025

@nicolas-grekas was this somewhere reverted? I see $hookedProperties but it's not used later in code https://github.com/symfony/symfony/blame/7.3/src/Symfony/Component/VarExporter/Internal/LazyObjectRegistry.php#L69

The part with foreach remained unchanged, unlike in this commit, and I can't figure out why. I'm getting Cannot unset private(set) property.. error in doctrine hydrator that leads to that part of code, where unset is being executed.

@Rixafy
Copy link

Rixafy commented Feb 22, 2025

I see that in 6.4 the commit is OK and $hookedProperties are used, but I'm getting error here

(Cannot modify private(set) property), which I don't know if it's var-exporter problem or doctrine orm problem, because I know that doctrine is not yet ready for hooked properties, so I solved it with protected(set) for now (because it's called from doctrine proxy object that extends mine).

@nicolas-grekas
Copy link
Member Author

Please open a new issue if you want this to be tracked.

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.

4 participants