Skip to content

For a resource, update its underlying data only if the API data is different than its current data. #37

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.

Sign up for GitHub

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

Open
yadavsaroj wants to merge 1 commit into
base: master
Choose a base branch
from
Open

For a resource, update its underlying data only if the API data is different than its current data. #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions spec/javascripts/deepMergeSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,29 @@ describe('deepMerge', function() {
expect(obj.a).toBe('bar');
});

it('should not override keys if values are same', function() {
var obj = {a: 'foo'};
spyOn(Ember, 'set');
Ember.Resource.deepMerge(obj, {a: 'foo'});
expect(Ember.set).not.toHaveBeenCalledWith(obj, 'a', 'foo');
});

it('should not override keys if values are empty arrays', function() {
var obj = {a: []};
spyOn(Ember, 'set');
Ember.Resource.deepMerge(obj, {a: []});
expect(Ember.set).not.toHaveBeenCalledWith(obj, 'a', []);
});

it('should override keys if values are not empty arrays', function() {
var obj = {a: [2]};
Ember.Resource.deepMerge(obj, {a: [3]});
expect(obj.a).toBeDefined();
expect(obj.a.length).toBe(1);
expect(obj.a[0]).toBe(3);
});


it('should leave other keys', function() {
var obj = {a: 'foo'};
Ember.Resource.deepMerge(obj, {b: 'bar'});
Expand Down
9 changes: 8 additions & 1 deletion src/ember-resource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,15 @@

if (Ember.typeOf(newValue) === 'object' && Ember.typeOf(oldValue) === 'object') {
Ember.Resource.deepMerge(oldValue, newValue);

} else {
Ember.set(objA, key, newValue);
oldValueEmptyArray = (Ember.typeOf(oldValue) == "array") && (oldValue.length == 0)
newValueEmptyArray = (Ember.typeOf(newValue) == "array") && (newValue.length == 0)

Copy link
Collaborator

@shajith shajith Mar 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use Em.compare to compare the two arrays (guarded by an isArray). There's also isEqual, which doesn't seem to work for arrays afaict.

Copy link
Contributor Author

@yadavsaroj yadavsaroj Mar 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not comparing the actual contents of an array, all I am checking is if they both are empty. Em.compare though can be used, seems to be an overkill for that.

btw just checked if Em.compare looks into actual array contents, unless I am missing something does not look like that as
Em.compare([{k1:1}], [{k1:111}]) returns 0, the value it returns if both objects being compared are equal.

Copy link
Collaborator

@shajith shajith Mar 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah compare can't be used here. So this is meant to support just empty arrays and scalars? (i.e it will skip arrays with values and objects)

Copy link
Contributor Author

@yadavsaroj yadavsaroj Mar 27, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! It won't compare non empty arrays.

if((newValue != oldValue) && (!oldValueEmptyArray && !newValueEmptyArray) ){
Ember.set(objA, key, newValue);
}

}
}
}
Expand Down