-
Notifications
You must be signed in to change notification settings - Fork 422
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
Get converters and .attr working the right way with nested objects #264
Comments
f9896d9 is where this went wrong. After fixing the logic, the question is what to do about this case:
The way its currently written, these owner attributes will be merged with what's currently in the owner observe. This means the actual data in the model store is overwritten, including the id and name. In this case we need to know about the fact that owner is a model instance and not overwrite. |
The above commit gets all the issues Josh was trying to fix working, but avoids calling __convert too early. I'm unsure if this is the right way to fix the above issue:
My logic is if the curVal is a model and the new one looks like it should be data for this model, call _set to let the converter handle it. I could probably come up with some situations where this wouldn't work, like when there is some very weird custom converter that isn't .model or .models on this instance and they want the data to be merged into the existing model. Thoughts? |
updated to not reference can.Model here: ae6b39c |
The expected behavior of .attr when passed a nested object is as follows:
myobj.attr('prop', obj);
This is not how it currently works, currently converters are called first, then _set or .attr is called, and converter is called again in _set. Fixing this logic will likely fix #208 and #174.
The text was updated successfully, but these errors were encountered: