Skip to content

Commit 6a42de3

Browse files
Scott TrinhScott Trinh
authored andcommitted
perf: Do not copy value on setItem
Uses `angular.extend` instead of `angular.copy` to make an object to remove `$promise` from. This keeps us from doubling the memory footprint on every save.
1 parent ac1c3b9 commit 6a42de3

2 files changed

Lines changed: 67 additions & 24 deletions

File tree

src/angular-localForage.js

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -113,34 +113,34 @@
113113
throw new Error('If you set an array of keys, the values should be an array too');
114114
}
115115

116-
var promises = [];
117-
angular.forEach(key, function(k, index) {
118-
promises.push(self.setItem(k, value[index]))
119-
});
120-
121-
return $q.all(promises);
116+
return $q.all(key.map(function (k, index) {
117+
return self.setItem(k, value[index]);
118+
}));
122119
} else {
123120
var deferred = $q.defer(),
124121
args = arguments,
125-
localCopy = typeof Blob !== 'undefined' && typeof ArrayBuffer !== 'undefined' && (value instanceof Blob || value instanceof ArrayBuffer) ? value : angular.copy(value);
122+
localCopy = value;
126123

127124
//avoid $promises attributes from value objects, if present.
128125
if(angular.isObject(localCopy) && angular.isDefined(localCopy.$promise)) {
129-
delete localCopy.$promise; //delete attribut from object structure.
126+
localCopy = angular.extend({}, value);
127+
delete localCopy.$promise;
130128
}
131129

132-
self._localforage.setItem(self.prefix() + key, localCopy).then(function success() {
133-
if(notify.setItem) {
134-
$rootScope.$broadcast('LocalForageModule.setItem', {
135-
key: key,
136-
newvalue: localCopy,
137-
driver: self.driver()
138-
});
139-
}
140-
deferred.resolve(localCopy);
141-
}, function error(data) {
142-
self.onError(data, args, self.setItem, deferred);
143-
});
130+
self._localforage.setItem(self.prefix() + key, localCopy)
131+
.then(function success() {
132+
if(notify.setItem) {
133+
$rootScope.$broadcast('LocalForageModule.setItem', {
134+
key: key,
135+
newvalue: localCopy,
136+
driver: self.driver()
137+
});
138+
}
139+
deferred.resolve(localCopy);
140+
})
141+
.catch(function withError(error) {
142+
self.onError(data, args, self.setItem, deferred);
143+
});
144144

145145
return deferred.promise;
146146
}

tests/angular-localForage.js

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,19 @@ describe('Module: LocalForageModule', function() {
183183

184184
it('setItem and getItem should work', function(done) {
185185
var interval = triggerDigests();
186+
var objectToStore = {
187+
name: 'Scott Trinh'
188+
};
186189

187-
$localForage.setItem('myName', 'Olivier Combe').then(function(data) {
188-
expect(data).toEqual('Olivier Combe');
190+
spyOn($localForage._localforage, 'setItem').and.callThrough();
189191

190-
$localForage.getItem('myName').then(function(data) {
192+
$localForage.setItem('myObject', objectToStore).then(function(data) {
193+
expect(data).toEqual({ name: 'Scott Trinh' });
194+
expect($localForage._localforage.setItem.calls.mostRecent().args[1]).toBe(objectToStore);
195+
196+
$localForage.getItem('myObject').then(function(data) {
191197
stopDigests(interval);
192-
expect(data).toEqual('Olivier Combe');
198+
expect(data).toEqual({ name: 'Scott Trinh' });
193199
done();
194200
}, done);
195201

@@ -448,6 +454,43 @@ describe('Module: LocalForageModule', function() {
448454
}).toThrowError('You must define a key to set');
449455
});
450456

457+
it('setItem should remove $promise if present', function (done) {
458+
var interval = triggerDigests();
459+
var objectToStore = {
460+
$promise: {},
461+
childObject: {}
462+
};
463+
var objectNoPromise = {
464+
noPromise: {}
465+
};
466+
spyOn($localForage._localforage, 'setItem').and.callThrough();
467+
468+
$localForage.setItem(['myObject', 'noPromise'], [objectToStore, objectNoPromise]).then(function() {
469+
var setWith = $localForage._localforage.setItem.calls.argsFor(0)[1];
470+
var setWithNoPromise = $localForage._localforage.setItem.calls.argsFor(1)[1];
471+
expect(setWith).not.toBe(objectToStore);
472+
expect(setWith.childObject).toBe(objectToStore.childObject);
473+
expect(setWith).toEqual({
474+
childObject: {}
475+
});
476+
expect(objectToStore).toEqual({
477+
$promise: {},
478+
childObject: {}
479+
});
480+
481+
expect(setWithNoPromise).toBe(objectNoPromise);
482+
483+
$localForage.getItem('myObject').then(function(data) {
484+
stopDigests(interval);
485+
expect(Object.keys(data).length).toBe(1);
486+
expect(data).toEqual({
487+
childObject: {}
488+
});
489+
done();
490+
}, done);
491+
}, done);
492+
})
493+
451494
describe("bind", function() {
452495
var $scope, $q;
453496

0 commit comments

Comments
 (0)