-
Notifications
You must be signed in to change notification settings - Fork 476
Data: Support object as second argument #235
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.
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
Changes from all commits
43832ae
73ffd58
19a2516
96c5817
b29c94f
4151a22
6a4df76
5fa5460
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,26 @@ var oldData = jQuery.data; | |
jQuery.data = function( elem, name, value ) { | ||
var curData; | ||
|
||
// Name can be an object, and each entry in the object is meant to be set as data | ||
if ( name && typeof name === "object" && arguments.length === 2 ) { | ||
curData = jQuery.hasData( elem ) && oldData.call( this, elem ); | ||
var sameKeys = {}; | ||
for ( var key in name ) { | ||
if ( key !== jQuery.camelCase( key ) ) { | ||
migrateWarn( "jQuery.data() always sets/gets camelCased names: " + key ); | ||
curData[ key ] = name[ key ]; | ||
} else { | ||
sameKeys[ key ] = name[ key ]; | ||
} | ||
} | ||
|
||
oldData.call( this, elem, sameKeys ); | ||
|
||
return name; | ||
} | ||
|
||
// If the name is transformed, look for the un-transformed name in the data object | ||
if ( name && name !== jQuery.camelCase( name ) ) { | ||
if ( name && typeof name === "string" && name !== jQuery.camelCase( name ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this change is necessary. We only support documented scenarios, while we want to add support for the object as the second parameter here at this point we know it's not an object so we're fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with your reason, which is why I think it should goes straight to jQuery's own .data method, and will throw the same error as when jquery-migrate is not used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
curData = jQuery.hasData( elem ) && oldData.call( this, elem ); | ||
if ( curData && name in curData ) { | ||
migrateWarn( "jQuery.data() always sets/gets camelCased names: " + name ); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
oldData.apply( this, arguments )
similarly to how what the other configuration is handled? You wouldn't have to recordsameKeys
at all then.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sameKeys is not the same as the second argument, as it only contains keys that are in camelCase and thus does not need to be handled here.
The other oldData.apply is actually doing the same if it determines the key is already camel case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, I was comparing it to the second part but there's an early return there if the non-camelCased version of the key was found. It's fine then. 👍