Skip to content

[Serializer] Fix type error not be accessed before initialization #50776

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
Jun 27, 2023

Conversation

shyim
Copy link
Contributor

@shyim shyim commented Jun 26, 2023

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets shopware/shopware#3190
License MIT
Doc PR

Make the property like in Symfony 6.2, with 6.3 it got the type hint but the default value was missing

@xabbuh
Copy link
Member

xabbuh commented Jun 26, 2023

I fail to see how the error can actually happen as this property is initialised inside the constructor.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 26, 2023

I guess it can happen when the constructor is not called by a child class. Which child class are we talking about? Isn't this an issue with the child class?

@shyim
Copy link
Contributor Author

shyim commented Jun 26, 2023

I think the problem comes from old cache items, without null on the typed property it gets to uninitialized. Always hard to add types to serialised objects 😅 . We have no class in Shopware which extends that class

https://3v4l.org/ajENt

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

That's a risk we need to remember when adding types to properties!

@nicolas-grekas
Copy link
Member

Thank you @shyim.

@nicolas-grekas nicolas-grekas merged commit 7d549df into symfony:6.3 Jun 27, 2023
@fabpot fabpot mentioned this pull request Jul 30, 2023
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