data icon indicating copy to clipboard operation
data copied to clipboard

Fix: @attr defaultValue() results should persist after initialization

Open christophersansone opened this issue 10 months ago • 2 comments

Description

Fixes #9315.

(cc @runspired)

Notes for the release

Before this change, when @attr defines a defaultValue as a function, the default value instantiated on a model instance will not necessarily be the value that is serialized. If the attribute was not set, and therefore the default value is used, then the serialization would call the defaultValue function again instead of returning the value on the model. This is mostly a problem when the attribute value is an array or object, because changes can be made to the object, resulting in those changes being lost upon serialization. With this change, the original defaultValue is persisted at the cache level to avoid this issue.

christophersansone avatar Apr 12 '24 23:04 christophersansone

Sorry! The day job got in the way this week. Planning to spend some time on it this weekend.

christophersansone avatar Apr 19 '24 18:04 christophersansone

@runspired Sorry for the delay!

Okay, I added the change to patchLocalAttributes, which I am pretty sure is correct. I also wrote a few tests that I believe cover the basics here, really just focused on when the default values are cached and / or regenerated. Thanks for pointing me in the right direction on these!

I did not figure out how to adequately test the change for patchLocalAttributes. If the server informed us of an attribute, the server can't really clear the entire state of that attribute. The attribute can either be specified in attributes or not. If it's specified, that specified value will be used. If it's not specified, the existing local value will be retained.

Let me know if there's anything else that makes sense to test.

christophersansone avatar Apr 30 '24 21:04 christophersansone

@runspired Is this something you think could be easily backported into 4.12?

daniellubovich avatar May 14 '24 15:05 daniellubovich

@daniellubovich the backport commit shouldn't be too hard

runspired avatar May 15 '24 05:05 runspired