Skip to content

(In)correct usage of .data() to set data attributes should be clarified in doc #1023

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

Closed
coldice opened this issue Feb 9, 2017 · 7 comments
Closed

Comments

@coldice
Copy link

coldice commented Feb 9, 2017

Referring to the simpler data function (http://api.jquery.com/data/) The behavior of the .data() function when setting data should be described more precisely, especially the difference between setting a html5 data attribute (properly) with .attr() and (not properly) with .data().

I was about to file a bug report with this sample: https://jsfiddle.net/wfh1ppxv/ when I found following this closed issue jquery/jquery#3158 it is intended behavior?

It is correct that the documentation does not say that calling .data("key", "value") will result in the html's DOM element data attribute set with data-key="value", but because of:

  • the name of the function
  • the following example to read the data
  • the description for the read function "Return the value at the named data store for the first element in the jQuery collection, as set by data(name, value) or by an HTML5 data-* attribute."
  • the references to the W3 specification for the dataset API

I find it very easy to think you could set the DOM data attribute with the .data() function. I suggest there should be a small note on the section for the set data function, pointing to .attr().

@mgol
Copy link
Member

mgol commented Feb 9, 2017

The docs at https://api.jquery.com/data/#data-html5 say:

The data- attributes are pulled in the first time the data property is accessed and then are no longer accessed or mutated (all data values are then stored internally in jQuery).

Do you have a suggestion how to improve that?

@coldice
Copy link
Author

coldice commented Feb 9, 2017

I think this part is okay, you just have to read the documentation carefully. My issue was only with the .data() function to set data, while in the other linked issue the author had trouble reading the data. But in general, I think the confusion in both cases results from the fact that the function to set does not work like the function to get.

I would put a note at the end or below the first Definition ("Store arbitrary data associated with the matched elements [...]") like:

Note: To set html5 data attributes (eg. data-toggle='modal'), please refer to the .attr() function.

Or: The object's attributes in the DOM will not be set or changed by this function.

@EricBalingit
Copy link

EricBalingit commented Sep 22, 2018

This just feels dirty to me:

<!-- some arbitrary tag, note "true" on data-junk -->
<div id="test" data-junk="true">

<script>
    // and my code would like to
    var test = $ ( "#test" );
    test.data ( "junk", false );
    console.log ( Object.keys ( test.data () ) );
    // > ["junk"]
    console.log ( test.data ().junk );
    // > false
</script>

.data () had a well defined purpose originally, why allow it to collide with html5 data attributes?

Seems a separate function would have made more sense. I dunno .dattr() or something.

@dmethvin
Copy link
Member

Its easy to complain years later about a clearly bad design decision, but a lot harder to create a time machine to go back and stop it, or go out and fix the thousands of pages that depend on it, or even to update the documentation to clarify the behavior. We'd love some help on one of the fixes, especially the time machine.

@katyp
Copy link

katyp commented Oct 22, 2018

Hi! It looks like the person who opened this bug didn't actually want a time machine, just a doc update :) I agree that this subject is very obscure and should be better documented.

@coldice suggested two very good options, though the first is clearer to me:

  1. Note: To set html5 data attributes (eg. data-toggle='modal'), please refer to the .attr() function.
  2. The object's attributes in the DOM will not be set or changed by this function.

They also specified exactly which page needs the change: http://api.jquery.com/data/

Is there any feedback on those two suggestions, @mgol or @dmethvin ? It might be helpful to provide that and then green light the doc change, so that the issue doesn't stagnate and the docs can be updated.

Regardless of if issues need an "OK" before devs contribute a PR for them, it's always nice to hear back after posing two potential solutions to a problem!

Cheers :)

@coldice
Copy link
Author

coldice commented Oct 22, 2018

Indeed, I did not want to discuss the actual behavior or implementation of the functionality - which I think is fine. The reason for posting this was that I spent a few hours trying to fix (and first understand) a bug, which turned out to be a feature misunderstanding.

I had to re-read the documentation multiple times and do a few tests to find that my misunderstanding back then was:
You can set and read arbitrary data using .data, and you can read real data attributes, but you can not write them. It is a nice convenience feature to be able to read them, but I made the wrong assumption that this would also allow me to write them.

The items in the first post list, what lead me to this assumption. Of course today, I find it obvious, because I know .data() doesn't change the DOM attributes.
But: If you google for "jquery write data attribute", it points you to the .data() page (and a few SO results from people making the same wrong assumption as I did). Even though it has been some time, I would still propose to add a little note pointing to the .attr() page. The 1. mentioned by @katyp looks fully sufficient for me.

@gibson042
Copy link
Member

I'm convinced: #1112

mgol pushed a commit that referenced this issue Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants