add support for ember-data-model-fragments #28
Conversation
addon/mixins/copyable.js
Outdated
| @@ -154,14 +154,14 @@ export default Mixin.create({ | |||
| ) { | |||
| let value = this.get(name); | |||
|
|
|||
| if (Copyable && Copyable.detect(value)) { | |||
| if ((Copyable && Copyable.detect(value)) || (value && value._isFragment)) { | |||
There was a problem hiding this comment.
Are all model fragments copyable?
There was a problem hiding this comment.
They are. For some reason Copyable.detect didn't seem to work.
addon/mixins/copyable.js
Outdated
| // "value" is an Ember.Object using the ember-copy addon | ||
| // (ie. old deprecated Ember.Copyable API - if you use | ||
| // the "Ember Data Model Fragments" addon and "value" is a fragment or | ||
| // if use your own serializer where you deserialize a value to an | ||
| // Ember.Object using this Ember.Copyable API) | ||
| value = value.copy(deep); | ||
| } else { | ||
| } else if (value) { |
There was a problem hiding this comment.
What is the reason for this change? Previously all values should be run through their transforms even if falsy. For example, you can have a vale of NaN that can have a custom transform to be Not a Number or 0, etc...
There was a problem hiding this comment.
It's to prevent serializing a null fragment value since that throws an exception. I can change the check to make sure its not a fragment to prevent reaching this branch in that case.
There was a problem hiding this comment.
I can change the check to make sure its not a fragment to prevent reaching this branch in that case.
Yeah, I would rather the check be explicit.
|
Looks like the CI failure is from a new ember-data. I'm not quite sure if its from my changes |
|
All good, Just beta and canary are failing. Changes look good. Thanks for taking this up @jakesjews! |
|
Released with |
|
thanks! |
Fix copying model fragments. Resolves #23