Skip to content

store.previousAttributes is storing references to nested objects, preventing changes from being detected #262

Closed
@OverZealous

Description

@OverZealous

This took some research to track down, but I'm pretty sure the mout deepMixin functionality has a flaw when copying nested objects. It's in this function here:

    function copyProp(val, key) {
        var existing = this[key];
        if (isPlainObject(val) && isPlainObject(existing)) {
            deepMixIn(existing, val);
        } else {
            this[key] = val;
        }
    }

In this function, this is the target object, and val is coming from the source object. The issue is that it only recursively processes nested objects if the key already exists on the target object. Otherwise, it simply stores a reference.

So what happens is the previousAttributes object is actually made of references to nested children. So, if you update a value on that nested child, it won't see it as a change (because the nested object is a literal reference).

I don't know if the right fix is to fix it on mout, which could have adverse affects for anyone using the library, or replace deepMixin with the existing angular.copy function, which already performs the required action without the extra dependency.

Edit: There is one difference between the functions, angular.copy only copies one item onto one destination, while deepMixin accepts multiple objects, if that matters.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions