broadway-serialization icon indicating copy to clipboard operation
broadway-serialization copied to clipboard

Updated: Use static when calling deserializationCallback()

Open karpilin opened this issue 5 years ago • 2 comments

An old PR from @MarkRedeman has been rebased on latest master.

@matthiasnoback This is both a proof of a problem and an actual fix. I'd very much like to see this merged.

karpilin avatar Feb 03 '20 22:02 karpilin

Coverage Status

Coverage remained the same at 100.0% when pulling fd5316f9f01fa0f7769072d17d8c10eed8c2a696 on karpilin:use-static-when-deserializing into 31497c1b75a48697544fe6312e2727f1ad412769 on matthiasnoback:master.

coveralls avatar Feb 03 '20 22:02 coveralls

Thanks for bringing this up. However, as I stated in the original PR, I wonder if you couldn't just use the trait in the child classes instead of the parent class. The result of using the trait in the parent class is predictable according to the properties of the PHP language, but will surely be a big surprise to many people. Also, there's no way to warn a user about the fact that private properties will be skipped. The warning is in the test, but not in production code, and when a user encounters this issue, they will probably spend quite some time debugging it.

matthiasnoback avatar Feb 04 '20 07:02 matthiasnoback