From 66518d10b363ba37fa6255f488118127d846e609 Mon Sep 17 00:00:00 2001 From: Oleg Gaidarenko Date: Fri, 13 Nov 2015 15:17:23 +0300 Subject: [PATCH] Revert "Data: always camelCase keys in .data()" This reverts commit 0e790985a76fd813a6e56696c87abeed5a6cf63b. --- src/data.js | 38 ++++++++++++++++----- src/data/Data.js | 86 +++++++++++++++++++++++++++++++++-------------- test/unit/data.js | 39 ++++----------------- 3 files changed, 95 insertions(+), 68 deletions(-) diff --git a/src/data.js b/src/data.js index d2855dde1..17e9c64f1 100644 --- a/src/data.js +++ b/src/data.js @@ -112,7 +112,7 @@ jQuery.fn.extend( { } return access( this, function( value ) { - var data; + var data, camelKey; // The calling jQuery object (element matches) is not empty // (and therefore has an element appears at this[ 0 ]) and the @@ -120,17 +120,24 @@ jQuery.fn.extend( { // will result in `undefined` for elem = this[ 0 ] which will // throw an exception if an attempt to read a data cache is made. if ( elem && value === undefined ) { - // Attempt to get data from the cache - // The key will always be camelCased in Data + // with the key as-is data = dataUser.get( elem, key ); if ( data !== undefined ) { return data; } + camelKey = jQuery.camelCase( key ); + // Attempt to get data from the cache + // with the key camelized + data = dataUser.get( elem, camelKey ); + if ( data !== undefined ) { + return data; + } + // Attempt to "discover" the data in // HTML5 custom data-* attrs - data = dataAttr( elem, key ); + data = dataAttr( elem, camelKey, undefined ); if ( data !== undefined ) { return data; } @@ -140,11 +147,24 @@ jQuery.fn.extend( { } // Set the data... - this.each( function() { - - // We always store the camelCased key - dataUser.set( this, key, value ); - } ); + camelKey = jQuery.camelCase( key ); + this.each(function() { + // First, attempt to store a copy or reference of any + // data that might've been store with a camelCased key. + var data = dataUser.get( this, camelKey ); + + // For HTML5 data-* attribute interop, we have to + // store property names with dashes in a camelCase form. + // This might not apply to all properties...* + dataUser.set( this, camelKey, value ); + + // *... In the case of properties that might _actually_ + // have dashes, we need to also store a copy of that + // unchanged property. + if ( key.indexOf("-") > -1 && data !== undefined ) { + dataUser.set( this, key, value ); + } + }); }, null, value, arguments.length > 1, null, true ); }, diff --git a/src/data/Data.js b/src/data/Data.js index e214282b3..b33f13497 100644 --- a/src/data/Data.js +++ b/src/data/Data.js @@ -12,7 +12,33 @@ Data.uid = 1; Data.prototype = { - cache: function( owner ) { + register: function( owner, initial ) { + var value = initial || {}; + + // If it is a node unlikely to be stringify-ed or looped over + // use plain assignment + if ( owner.nodeType ) { + owner[ this.expando ] = value; + + // Otherwise secure it in a non-enumerable, non-writable property + // configurability must be true to allow the property to be + // deleted with the delete operator + } else { + Object.defineProperty( owner, this.expando, { + value: value, + writable: true, + configurable: true + }); + } + return owner[ this.expando ]; + }, + cache: function( owner, initial ) { + // We can accept data for non-element nodes in modern browsers, + // but we should not, see #8335. + // Always return an empty object. + if ( !acceptData( owner ) ) { + return {}; + } // Check if the owner object already has a cache var value = owner[ this.expando ]; @@ -50,16 +76,14 @@ Data.prototype = { cache = this.cache( owner ); // Handle: [ owner, key, value ] args - // Always use camelCase key (gh-2257) if ( typeof data === "string" ) { - cache[ jQuery.camelCase( data ) ] = value; + cache[ data ] = value; // Handle: [ owner, { properties } ] args } else { - // Copy the properties one-by-one to the cache object for ( prop in data ) { - cache[ jQuery.camelCase( prop ) ] = data[ prop ]; + cache[ prop ] = data[ prop ]; } } return cache; @@ -67,12 +91,10 @@ Data.prototype = { get: function( owner, key ) { return key === undefined ? this.cache( owner ) : - - // Always use camelCase key (gh-2257) - owner[ this.expando ] && owner[ this.expando ][ jQuery.camelCase( key ) ]; + owner[ this.expando ] && owner[ this.expando ][ key ] }, access: function( owner, key, value ) { - + var stored; // In cases where either: // // 1. No key was specified @@ -85,9 +107,12 @@ Data.prototype = { // 2. The data stored at the key // if ( key === undefined || - ( ( key && typeof key === "string" ) && value === undefined ) ) { + ((key && typeof key === "string") && value === undefined) ) { + + stored = this.get( owner, key ); - return this.get( owner, key ); + return stored !== undefined ? + stored : this.get( owner, jQuery.camelCase(key) ); } // When the key is not a string, or both a key and value @@ -103,35 +128,44 @@ Data.prototype = { return value !== undefined ? value : key; }, remove: function( owner, key ) { - var i, + var i, name, camel, cache = owner[ this.expando ]; if ( cache === undefined ) { return; } - if ( key !== undefined ) { + if ( key === undefined ) { + this.register( owner ); + } else { // Support array or space separated string of keys if ( jQuery.isArray( key ) ) { - - // If key is an array of keys... - // We always set camelCase keys, so remove that. - key = key.map( jQuery.camelCase ); + // If "name" is an array of keys... + // When data is initially created, via ("key", "val") signature, + // keys will be converted to camelCase. + // Since there is no way to tell _how_ a key was added, remove + // both plain key and camelCase key. #12786 + // This will only penalize the array argument path. + name = key.concat( key.map( jQuery.camelCase ) ); } else { - key = jQuery.camelCase( key ); - - // If a key with the spaces exists, use it. - // Otherwise, create an array by matching non-whitespace - key = key in cache ? - [ key ] : - ( key.match( rnotwhite ) || [] ); + camel = jQuery.camelCase( key ); + // Try the string as a key before any manipulation + if ( key in cache ) { + name = [ key, camel ]; + } else { + // If a key with the spaces exists, use it. + // Otherwise, create an array by matching non-whitespace + name = camel; + name = name in cache ? + [ name ] : ( name.match( rnotwhite ) || [] ); + } } - i = key.length; + i = name.length; while ( i-- ) { - delete cache[ key[ i ] ]; + delete cache[ name[ i ] ]; } } diff --git a/test/unit/data.js b/test/unit/data.js index abd01e92e..fdd54629d 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -601,34 +601,6 @@ QUnit.test( ".data should not miss attr() set data-* with hyphenated property na assert.deepEqual( b.data( "long-param" ), { a: 2 }, "data with property long-param was found, 2" ); } ); -QUnit.test( ".data always sets data with the camelCased key (gh-2257)", function( assert ) { - assert.expect( 18 ); - - var div = jQuery( "
" ).appendTo( "#qunit-fixture" ), - datas = { - "non-empty": "a string", - "empty-string": "", - "one-value": 1, - "zero-value": 0, - "an-array": [], - "an-object": {}, - "bool-true": true, - "bool-false": false, - - // JSHint enforces double quotes, - // but JSON strings need double quotes to parse - // so we need escaped double quotes here - "some-json": "{ \"foo\": \"bar\" }" - }; - - jQuery.each( datas, function( key, val ) { - div.data( key, val ); - var allData = div.data(); - assert.equal( allData[ key ], undefined, ".data does not store with hyphenated keys" ); - assert.equal( allData[ jQuery.camelCase( key ) ], val, ".data stores the camelCased key" ); - } ); -} ); - QUnit.test(".data supports interoperable hyphenated/camelCase get/set of properties with arbitrary non-null|NaN|undefined values", function( assert ) { var div = jQuery( "
", { id: "hyphened" } ).appendTo( "#qunit-fixture" ), datas = { @@ -723,20 +695,20 @@ QUnit.test( ".data supports interoperable removal of properties SET TWICE #13850 } ); } ); -QUnit.test( ".removeData supports removal of hyphenated properties via array (#12786, gh-2257)", function( assert ) { - assert.expect( 4 ); +QUnit.test( ".removeData supports removal of hyphenated properties via array (#12786)", function( assert ) { + expect( 4 ); var div, plain, compare; div = jQuery( "
" ).appendTo( "#qunit-fixture" ); plain = jQuery( {} ); - // Properties should always be camelCased + // When data is batch assigned (via plain object), the properties + // are not camel cased as they are with (property, value) calls compare = { // From batch assignment .data({ "a-a": 1 }) - "aA": 1, - + "a-a": 1, // From property, value assignment .data( "b-b", 1 ) "bB": 1 }; @@ -751,6 +723,7 @@ QUnit.test( ".removeData supports removal of hyphenated properties via array (#1 div.removeData( [ "a-a", "b-b" ] ); plain.removeData( [ "a-a", "b-b" ] ); + // NOTE: Timo's proposal for "propEqual" (or similar) would be nice here assert.deepEqual( div.data(), {}, "Data is empty. (div)" ); assert.deepEqual( plain.data(), {}, "Data is empty. (plain)" ); } ); -- 2.39.5