Skip to content

[VarExporter] Add Hydrator::hydrate() and preserve PHP references when using it #46452

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 27, 2022
Merged

[VarExporter] Add Hydrator::hydrate() and preserve PHP references when using it #46452

merged 1 commit into from
May 27, 2022

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 24, 2022 •
edited
Loading

Q A
Branch? 6.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

To make it easy to hydrate already created objects (usually created without calling the constructor.)

  • Add Hydrator::hydrate()
  • Preserve PHP references also when using Hydrator::hydrate() or Instantiator::instantiate()
  • Add support for hydrating from native (array) casts

@carsonbot carsonbot added this to the 6.2 milestone May 24, 2022
@carsonbot carsonbot changed the title [DependencyInjection][ProxyManager] Use lazy-loading ghost object proxies when possible [DependencyInjection] [ProxyManager] Use lazy-loading ghost object proxies when possible May 24, 2022
@nicolas-grekas nicolas-grekas changed the title [DependencyInjection] [ProxyManager] Use lazy-loading ghost object proxies when possible [VarExporter] Add Hydrator::hydrate() and preserve PHP references when using it May 25, 2022
@nicolas-grekas
Copy link
Member Author

Does the psalm failure make sense to anyone?

@stof
Copy link
Member

stof commented May 25, 2022

This error is because Psalm sees the returned value as being of type object, and not as being of type T (it fails to see that the instantiation bypassing the constructor actually produces an instance of the class name passed as argument). Given the magic happening in that instantiation, I would say that this is totally fine (and won't be reported for next PRs, as our CI only reports new issues detecting by psalm, not all of them)

@nicolas-grekas nicolas-grekas mentioned this pull request May 25, 2022
Closed
@nicolas-grekas nicolas-grekas force-pushed the ve-hydrator branch 3 times, most recently from a7fe0e4 to ef25192 Compare May 27, 2022 09:11
@nicolas-grekas
Copy link
Member Author

/cc @symfony/mergers this PR is ready and blocking #46458 if you have some time for a review 🙏

@carsonbot carsonbot changed the title [VarExporter] Add Hydrator::hydrate() and preserve PHP references when using it [DependencyInjection] Add Hydrator::hydrate() and preserve PHP references when using it May 27, 2022
@nicolas-grekas nicolas-grekas merged commit 6cd0da0 into symfony:6.2 May 27, 2022
@nicolas-grekas nicolas-grekas mentioned this pull request May 27, 2022
Closed
@nicolas-grekas nicolas-grekas deleted the ve-hydrator branch May 27, 2022 13:01
@lyrixx
Copy link
Member

lyrixx commented May 29, 2022

Why does Carson bot changed the title? This is wrong to me

@Nyholm
Copy link
Member

Nyholm commented May 29, 2022

Carson listens to "updates" on the PR. It names the title according to the (grey) labels.

When this PR was merged, Nicolas did a push ("update").

@carsonbot carsonbot changed the title [DependencyInjection] Add Hydrator::hydrate() and preserve PHP references when using it [VarExporter] Add Hydrator::hydrate() and preserve PHP references when using it May 29, 2022
@nicolas-grekas
Copy link
Member Author

Labels updated

@fabpot fabpot mentioned this pull request Oct 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed VarExporter
Projects
None yet
Milestone
6.2
Development

Successfully merging this pull request may close these issues.
6 participants
@nicolas-grekas @stof @lyrixx @Nyholm @OskarStark @carsonbot