Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Avoid side-effects when calling jQuery.hasData
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
  • Loading branch information
rwaldron committed Apr 1, 2013
1 parent 1f530e2 commit 332a490
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/data.js
Expand Up @@ -21,11 +21,17 @@ function Data() {
Data.uid = 1;

Data.prototype = {
key: function( owner ) {
key: function( owner, options ) {
var descriptor = {},
// Check if the owner object already has a cache key
unlock = owner[ this.expando ];

// `readonly` calls from hasData, on owners with no key
// should not create new/empty cache records
if ( !unlock && (options && options.readonly) ) {
return null;
}

// If not, create one
if ( !unlock ) {
unlock = Data.uid++;
Expand Down Expand Up @@ -158,7 +164,7 @@ Data.prototype = {
},
hasData: function( owner ) {
return !jQuery.isEmptyObject(
this.cache[ this.key( owner ) ]
this.cache[ this.key( owner, { readonly: true }) ] || {}

This comment has been minimized.

Copy link
@gibson042

gibson042 Apr 1, 2013

Member

@rwldrn I assume it's this way for a reason, but why not leave .key alone and bring .hasData up next to it like return !jQuery.isEmptyObject( owner[ this.expando ] || {} )? Redundant, admittedly, but also smaller and faster.

This comment has been minimized.

Copy link
@rwaldron

rwaldron Apr 1, 2013

Author Member

I'll give a try, the result here was at the tail end of trying a few different things.

This comment has been minimized.

Copy link
@rwaldron

rwaldron Apr 1, 2013

Author Member
);
},
discard: function( owner ) {
Expand Down
11 changes: 11 additions & 0 deletions test/unit/data.js
Expand Up @@ -51,6 +51,17 @@ test( "jQuery._data & _removeData, expected returns", function() {
);
});

test( "jQuery.hasData no side effects", function() {
expect(1);
var obj = {};

jQuery.hasData( obj );

equal( Object.getOwnPropertyNames( obj ).length, 0,
"No data expandos where added when calling jQuery.hasData(o)"
);
});

function dataTests (elem) {
var oldCacheLength, dataObj, internalDataObj, expected, actual;

Expand Down

0 comments on commit 332a490

Please sign in to comment.