Bug Tracker

Modify

Ticket #7579 (closed bug: fixed)

Opened 3 years ago

Last modified 18 months ago

jQuery.data() truncates numbers taken from data-xxx attributes

Reported by: dmethvin Owned by: dmethvin
Priority: high Milestone: 1.8
Component: data Version: 1.4.4
Keywords: Cc:
Blocking: Blocked by:

Description

Test case:  http://jsfiddle.net/dmethvin/8SsWK/

Discussion:  http://forum.jquery.com/topic/data-type-conversion

I found an issue with using .data() to access data-* attributes and how it does automatic type conversion. The conversion will break any string that just happens to look like a JavaScript value. A simple example:

HTML:

<div id="foo" data-version="1.0">...</div>

JS:

jsversion = $("#foo").data("version");

jsversion is now the number 1. That is totally wrong if you intend to use that attribute as a string. (I was using it to create a URL where "1.0" is the valid value.)

You can get around this with direct access: $("#foo").attr("data-version") but that's not as much fun. Also, the process is not reversible which is confusing:

$("#foo").data("foo", "1.0"); val = $("#foo").data("foo"); val is the string "1.0" and not converted to number 1. passing in 1.0 vs "1.0" would of course result in 1.

The data() docs say "Note that strings are left intact while JavaScript values are converted to their associated value (this includes booleans, numbers, objects, arrays, and null)". That seems poorly worded since "1.0" is a string just as much as "true", "false", or "null". It should say every attempt is made to convert the string to a JavaScript value otherwise it is left as a string. It would be nice to also mention that if you need literal strings you must use the attr() method mentioned above.

I'd suggest the automatic conversion be removed and let users deal with it as needed. Perhaps add a helper function or alternate data() function to do conversion. The way it works now is intentional and has test cases but I'm unsure what the motivation of this conversion is in the first place? Would it break a common use case to just always treat attributes as strings?

Change History

comment:1 Changed 3 years ago by dmethvin

I've updated the test case with a few more examples. Based on these I think we'd be better to back off the magic for all but {} and [], returning everything else as string.

comment:2 Changed 3 years ago by dmethvin

  • Priority changed from undecided to high
  • Status changed from new to open
  • Component changed from unfiled to data
  • Milestone changed from 1.5 to 1.4.5

comment:3 Changed 3 years ago by anonymous

If one would implement a getter for the "data-" attribute the way W3C proposes, the getter would have to return a string.

Quoting the W3C HTML5 Working Draft:
«Except where otherwise specified, attributes on HTML elements may have any string value, including the empty string. Except where explicitly stated, there is no restriction on what text can be specified in such attributes.»
 http://www.w3.org/TR/html5/elements.html#custom-data-attribute

comment:4 Changed 3 years ago by dmethvin

  • Owner set to dmethvin
  • Status changed from open to assigned

comment:5 Changed 3 years ago by dmethvin

#7231 is a duplicate of this ticket.

comment:6 Changed 3 years ago by dmethvin

comment:7 Changed 3 years ago by john

  • Status changed from assigned to closed
  • Resolution set to wontfix

Uhhh... what? This is a major regression! As mentioned in other tickets - if you want the actual precise value just do: .attr("data-version").

comment:8 Changed 3 years ago by john

  • Status changed from closed to reopened
  • Resolution wontfix deleted

comment:9 Changed 3 years ago by john

So, I guess we should definitely discuss this more because 1.5 is a time during which we could change thing (although it'll definitely break code).

comment:10 Changed 3 years ago by dmethvin

  • Keywords needsreview added
  • Milestone changed from 1.4.5 to 1.5

comment:11 Changed 3 years ago by jitter

#7742 is a duplicate of this ticket.

comment:12 Changed 3 years ago by armloch@…

big problem for me. my "numbers" are too large and parseInt() doesn't parse them correctly:

var foo = "1283767437056205165" parseInt(foo) 1283767437056205000

-

simon

comment:13 Changed 3 years ago by paul.irish

I have updated the docs for $.fn.data to Dave's suggested text.  http://api.jquery.com/data/

It now reads:

Every attempt is made to convert the string to a JavaScript value (this includes booleans, numbers, objects, arrays, and null) otherwise it is left as a string. If you need literal strings you must use the attr() method.

comment:14 Changed 3 years ago by jdub@…

Found a related problem. Here's the HTML as rendered (not set up by jQuery):

<a class=​"screen-name username" data-screen-name=​"0xDECAFB4D" href=​"https:​/​/​twitter.com/​0xDECAFB4D" title=​"mls" target=​"_blank">​0xDECAFB4D​</a>

Then at the console:

$('a[title=mls]').data('screen-name')
0

$('a[title=mls]').attr('data-screen-name')
"0xDECAFB4D"

Thanks. The suggestions to only add smarts to the [ and { cases sound pretty sensible to me. :-)

comment:15 Changed 3 years ago by dmethvin

  • Keywords needsdocs added; needsreview removed
  • Status changed from reopened to closed
  • Resolution set to wontfix

comment:16 Changed 3 years ago by dmethvin

If you want the data attributes as entered in the HTML, use .attr("data-whatever") instead.

comment:17 Changed 3 years ago by dmethvin

  • Keywords needsdocs removed

comment:18 Changed 3 years ago by jitter

#8042 is a duplicate of this ticket.

comment:19 Changed 3 years ago by embrangler

Ugh. I disagree with wontfix-ing this.

It's ok if .data() doesn't work for large ints, but it shouldn't pretend to be working and return wrong values.

This text mentioned in comment 13 is misleading:

Every attempt is made to convert the string to a JavaScript value (this includes booleans, numbers, objects, arrays, and null) otherwise it is left as a string.

For large ints the result is a WRONG integer value. I would expect it to be a string, since it fails to convert properly.

Last edited 3 years ago by embrangler (previous) (diff)

comment:20 Changed 3 years ago by jitter

#8356 is a duplicate of this ticket.

comment:21 Changed 3 years ago by addyosmani

#8435 is a duplicate of this ticket.

comment:22 Changed 3 years ago by ajpiano

#8709 is a duplicate of this ticket.

comment:23 Changed 3 years ago by awensley@…

I also think not fixing this is a mistake. What use is .data() if you can't trust the data it returns?

I guess I just can't use .data() for anything anymore then because I never know what values might trigger this bug. Bummer.

comment:24 Changed 3 years ago by mattw@…

This would be less annoying if we could quote the string-that-looks-like-a-JS-value and have that work. But I just tried that, and it leaves the quotes in.

Using .attr("data-xxx") is problematic if the intent was to use the attribute to initialize the value at page load, then use .data to update the value at a later time. Now you no longer have a unified way to access the current value.

I guess I'll prefix and substring as my workaround.

comment:25 Changed 3 years ago by jyrki@…

The implicit conversion of .data causes too much problems. A few example:

  "01234" -> 1234
  "74e2" -> 7400

We're constantly having problems with this and now we are resorting in prefixing it. We can't use attr as we generate data attributes in mustache templates and modify them later on with .data. The data is random "hexadecimal" strings, so that's why we're running in severe problems with this.

This ticket should be reopened and the implicit behavior of the data function reconsidered

comment:26 Changed 3 years ago by rwaldron

#9688 is a duplicate of this ticket.

comment:27 Changed 3 years ago by anonymous

Automatic type conversion is a really bad idea and should be removed from jQuery.

I was storing some IDs as data attributes. IDs are strings not numbers - although they look like numbers. When I access them with the data() function they are cast as numbers and become corrupted because integers in JavaScript are only considered accurate up to 15 digits.

jQuery shouldn't be treating developers as idiots and trying to take control of their programs. If you want to have type conversion, have it as an option, but it should not be the default behaviour.

comment:28 Changed 2 years ago by anonymous

ok, this is completely massed up, why on earth would one do this? Is jQuery catering to kids? Stupid thing was converting my product codes with leading zeros to integers... so 00800 ended up 800... you should leave conversion up to developers...

had to change

var prodCode = $(itemBlock).data("prodcode");

to

var prodCode = $(itemBlock).attr("data-prodcode");

comment:29 Changed 2 years ago by rwaldron

Unfortunately, before jQuery.fn.data() was open to the public, it was a core-use-only function that had nothing to do with HTML5 data-attrs (predates them), and as such, it stored only real JavaScript values - again, nothing to do with HTML5 data-attrs. Then one day it was given to the public... then soon after the public wanted it to interface with data-attrs, so jQuery allowed it, but the function still had to support all of the extant code using the original jQuery.fn.data() behaviour.

This explanation could've been derived by reading the above thread, so next time, try to be a little more understanding and a little less aggressive.

comment:30 Changed 2 years ago by dmethvin

I think we've explained the use of the .data() method pretty clearly here, I hope it helps:

 http://www.learningjquery.com./2011/09/using-jquerys-data-apis

TL;DR: If you simply want a string, use .attr() -- data attributes *are* attributes after all

comment:31 Changed 2 years ago by dmethvin

#10856 is a duplicate of this ticket.

comment:32 Changed 2 years ago by jdmarshall

The thread is basically apologizing for not following the spec, and for losing data precision. Marking it "won't fix" is bound to earn you "aggressive" feedback. I understand, now, how it got to be that way, and that's very interesting. However, understanding is not the same as saying "It's okay". This is definitely NOT okay.

There has to be a way to share code without making both bits (mis)function identically.

comment:33 Changed 2 years ago by jursetto

One trick to make sure your data isn't converted is to pass a quoted string inside a 1-element array, then reference that element. This will ensure the JSON parser is engaged because it starts with [. It also lets you pass arbitrary objects you do want converted (in which case you can't use .attr()).

Example:

<input id="abc" data-foo='["true"]' data-bar='[true]'>

$('#abc').data('foo')[0] -> "true"
$('#abc').data('bar')[0] -> true

comment:34 Changed 2 years ago by rwaldron

Or...

data-foo="true"

$('#abc').attr('data-bar') -> "true"

... as shown above

comment:35 Changed 2 years ago by jursetto

That's right, but you skipped the second part of my comment; with attr() you can't accept either a boolean value OR a string, whereas you can with the technique above. Ticket #7231 had some people asking for the ability to do both, although it looks like this ticket didn't.

comment:36 Changed 2 years ago by dmethvin

#11060 is a duplicate of this ticket.

comment:37 Changed 2 years ago by anonymous

Wow, this is so beyond absurd.

Basically, you have a whole group of USERS who are asking you to fix this, with valid reasons, and you are basically ignoring them, saying to just use another method. Even though there is a perfectly acceptable, backwards compatible solution to this problem.

I can't even begin to understand the mentality behind this type of decision. dmethvin, are you just hard headed?

comment:38 Changed 2 years ago by dmethvin

Yes, I am hard headed if I see that changing documented interfaces will cause a lot of trouble. If I had a time machine I could go back and redefine this interface to be more compatible with the HTML5 spec that was not yet written, but I do not.

You feel passionately about changing this because you have no existing code that would break. Imagine how mad you would be if you had deployed code based on the current behavior and we changed it.

To use your pent-up energy, try writing a patch that makes it behave the way you ideally would like it to, but does not break old code. Or, just use .attr() which works fine.

comment:39 Changed 2 years ago by david@…

Would a patch be considered which allows quoted strings? For example, so that: … data-foo='"123"' … could be used to force a string?

For example, changing the existing logic:

	data = data === "true" ? true :
		data === "false" ? false :
		data === "null" ? null :
		jQuery.isNumeric( data ) ? parseFloat( data ) :
		rbrace.test( data ) ? jQuery.parseJSON( data ) :
		data;

To move the rbrace.test call above isNumeric, and modifying it to include ":

	var jsonDataTest = /^(?:\{.*\}|\[.*\]|".*")$/
	...
	data = data === "true" ? true :
		data === "false" ? false :
		data === "null" ? null :
		jsonDataTest.test( data ) ? jQuery.parseJSON( data ) :
		jQuery.isNumeric( data ) ? parseFloat( data ) :
		data;

This would guarantee (I believe) that any JSON value in a data- property would be correctly encoded, and it would provide a safe (and probably backwards compatible?) mechanism for storing strings-which-look-like-numbers.

comment:40 Changed 2 years ago by rwaldron

Use attr

comment:42 Changed 2 years ago by draeton@…

Considering the lack of will to patch jQuery, here's a simple plugin:

(function ($) {

    "use strict";

    var dataTest = /^data-/;

    var dashesToCamelCase = function (name) {
            return name.replace(dataTest, "").replace(/-\w/g, function (str) {
                return str.slice(1).toUpperCase();
            });
        };

    $.fn.getData = function () {
        var data = {};
        var el = this.get(0);
        var attr = el.attributes;
        var name;
        var i, l;

        for (i = 0, l = attr.length; i < l; i++) {
            if (dataTest.test(attr[i].name)) {
                name = dashesToCamelCase(attr[i].name);
                data[name] = attr[i].nodeValue;
            }
        }

        return data;
    };

}(jQuery));

 https://gist.github.com/1928836

comment:43 Changed 2 years ago by dmethvin

#11445 is a duplicate of this ticket.

comment:44 Changed 2 years ago by anonymous

Better to fix this now than to have loads of more people continue to implement this poor solution. I'm sure more people are running into problems with their code right now than than would be affected by the change because they are counting on this automatic type conversion. Worse yet, many don't know about the problem till it hits them some time in the future when they least expect it.

comment:45 Changed 2 years ago by rwaldron

Can't be fixed without breaking core functionality, posting more comments won't change fact

Last edited 2 years ago by rwaldron (previous) (diff)

comment:46 Changed 23 months ago by dvirazulay

Are you serious? How about not supporting those users that used .data, thinking it will be treated as a string (like it should have been) and suddenly when their numbers grow over a certain point, or one decides to enter a number that will be truncated/rounded due to this and gets the wrong behavior out of it? This will be a real mess to debug.

If no one can come up with a real reason why this might break old code, why not implement it? There was enough time to think of a valid reason not to fix this huge flaw, which makes .data obsolete and not reliable.

comment:47 Changed 23 months ago by dmethvin

@dvirazulay, it would never break old code because the .data() API existed before HTML5 defined data- att ... aw, just shut up and read this article.

 http://www.learningjquery.com/2011/09/using-jquerys-data-apis

comment:48 Changed 23 months ago by gibson042

@dvirazulay, feel free to plugin your desired behavior with a gist:  https://gist.github.com/2861953. Same goes for those passionate about #11297 and #10174.

comment:49 Changed 22 months ago by dmethvin

#12052 is a duplicate of this ticket.

comment:50 Changed 22 months ago by rwaldron

#12060 is a duplicate of this ticket.

comment:51 Changed 21 months ago by Dave Methvin

  • Resolution changed from wontfix to fixed

Fix #7579. Don't convert to number if it changes the string. Close gh-852.

Net effect here is that hex numbers and most exponential-format numbers or long sequences of digits will remain strings rather than being coerced to numbers. `The people have spoken.

Changeset: ce15bd7d0c14106521bd21179b1507f2863d1960

comment:52 Changed 21 months ago by dmethvin

  • Milestone changed from 1.5 to 1.8

comment:53 Changed 20 months ago by timmywil

#12332 is a duplicate of this ticket.

comment:54 Changed 20 months ago by latchkey

I've been thinking a lot about this for the last couple of days and have come to the conclusion that this fix is a terrible idea that only creates even more confusion in the long term. It really is a hack on top of a bad design decision in the first place. Now, the message is 'use .data() instead of .attr() because it is kinda safe to use in certain circumstances.'

If you generate a div such as <div data-foo="4.000"></div>, you get back a String... $('div').data('foo') despite the fact that the documentation says it tries to coerce numbers. If I do $('div').data('foo', 4.000) and then look at it with .data('foo'), it is a number. If I do data-foo="4000", it is a number. I'm sorry, but this is very inconsistent behavior.

comment:55 Changed 20 months ago by dmethvin

  • Keywords needsdocs added

Proposed addition to the docs:

A value is only converted to a number if doing so doesn't change the value's representation. For example, "1E02" and "100" are equivalent as numbers but not as strings, so "1E04" remains a string. A value like "1.4000" is not converted to number because the trailing zeroes would be stripped.

comment:56 Changed 18 months ago by mikesherov

  • Keywords needsdocs removed

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.