Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(jqLite): never add to the cache for non-element/document nodes
Browse files Browse the repository at this point in the history
Calling `jqLite.data()` on a disallowed node type caused an empty object to be added to the
cache. This could lead to memory leaks since we no longer clean up such node types when they are
removed from the DOM.

Closes #7966
  • Loading branch information
petebacondarwin committed Jun 25, 2014
1 parent 768a191 commit 91754a7
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 19 deletions.
37 changes: 18 additions & 19 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,30 +314,29 @@ function jqLiteExpandoStore(element, key, value) {
}

function jqLiteData(element, key, value) {
var data = jqLiteExpandoStore(element, 'data'),
isSetter = isDefined(value),
keyDefined = !isSetter && isDefined(key),
isSimpleGetter = keyDefined && !isObject(key);
if (jqLiteAcceptsData(element)) {
var data = jqLiteExpandoStore(element, 'data'),
isSetter = isDefined(value),
keyDefined = !isSetter && isDefined(key),
isSimpleGetter = keyDefined && !isObject(key);

if (!data && !isSimpleGetter) {
jqLiteExpandoStore(element, 'data', data = {});
}
if (!data && !isSimpleGetter) {
jqLiteExpandoStore(element, 'data', data = {});
}

if (isSetter) {
// set data only on Elements and Documents
if (jqLiteAcceptsData(element)) {
if (isSetter) {
data[key] = value;
}
} else {
if (keyDefined) {
if (isSimpleGetter) {
// don't create data in this case.
return data && data[key];
} else {
if (keyDefined) {
if (isSimpleGetter) {
// don't create data in this case.
return data && data[key];
} else {
extend(data, key);
}
} else {
extend(data, key);
return data;
}
} else {
return data;
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,23 @@ describe('jqLite', function() {
selected.removeData('prop2');
});


it('should not add to the cache if the node is a comment or text node', function() {
var calcCacheSize = function() {
var count = 0;
for (var k in jqLite.cache) { ++count; }
return count;
};

var nodes = jqLite('<!-- some comment --> and some text');
expect(calcCacheSize()).toEqual(0);
nodes.data('someKey');
expect(calcCacheSize()).toEqual(0);
nodes.data('someKey', 'someValue');
expect(calcCacheSize()).toEqual(0);
});


it('should emit $destroy event if element removed via remove()', function() {
var log = '';
var element = jqLite(a);
Expand Down

0 comments on commit 91754a7

Please sign in to comment.