Skip to content
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

Ticket: 8909 - $(element).data() will scan the attributes more times than needed. #335

Closed
wants to merge 2 commits into from

Conversation

gnarf
Copy link
Member

@gnarf gnarf commented Apr 19, 2011

If you make more than one call to $(element).data() you end up having to loop over the attributes of the element looking for "data-" attributes every time you access it. I'd like to propose adding a simple flag in our private _data() that can stop us from having to check more than once...'

This pull attempts to correct that using a private data value to store when it has parsed the elements attribs.

http://bugs.jquery.com/ticket/8909

@dmethvin
Copy link
Member

This looks like a good idea! It should make .data() a bit faster, the only drawback being if someone dynamically added a data- attr we wouldn't catch it. Seems like a small issue though.

@gnarf
Copy link
Member Author

gnarf commented Apr 19, 2011

Honestly, it seems that there is no support for "updated" attribs at the moment so it would kind of make sense that later appended ones don't show up either... I mean, if you change .data('someKeyFromAttrib', newValue) it doesn't update the html or anything, and the getter always returns whatever it read the first time... It doesn't re-read if it already cached the value...

@dmethvin
Copy link
Member

Yeah, it can get especially confusing if were to do $.data(elem, "abc") , $(elem).data("abc"), $.data(elem, "abc") because the middle call loads the cache. Oh well. It seems like this is a good idea in any case.

@dmethvin
Copy link
Member

dmethvin commented Aug 4, 2011

The pull looks good to me but I want to consult the jQuery Mobile team to be sure that this won't cause problems for them. They are set to use 1.6.3 as the base for their first release.

var attr = this[0].attributes, name;
for ( var i = 0, l = attr.length; i < l; i++ ) {
name = attr[i].name;
if ( elem.nodeType === 1 && !jQuery._data(elem, "parsedAttrs" ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jQuery._data(elem needs space at ( e

@gnarf
Copy link
Member Author

gnarf commented Aug 17, 2011

Great points @rwldrn - I'll re open when I've addressed these points and rebased

@gnarf gnarf closed this Aug 17, 2011
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants