Skip to main content

Bug Tracker

Side navigation

#12538 closed bug (notabug)

Opened September 14, 2012 10:25AM UTC

Closed September 14, 2012 12:54PM UTC

Last modified September 14, 2012 01:23PM UTC

HTML String instances not handled as string

Reported by: ilja.everila@liilak.com Owned by:
Priority: low Milestone: None
Component: selector Version: 1.8.1
Keywords: Cc:
Blocked by: Blocking:
Description

JQuery does not (up till 1.8.1 at least) handle String instances as strings, but objects:

    >>> $(new String('<div>asdf</div>'));
    [ String ]

This test causes said behaviour:

    // Handle HTML strings
    if ( typeof selector === "string" ) {

and should read

    // Handle HTML strings
    if ( typeof selector === "string" || selector instanceof String ) {

or any other testing method for instance of a prototype / class.

This may not be the most important feature in the world, but it did surprise us when using a factory producing String instances and trying to append them to DOM through jQuery.

Attachments (0)
Change History (8)

Changed September 14, 2012 10:36AM UTC by ilja.everila@liilak.com comment:1

As JSLint for example warns against using String as constructor, this might not be desired behaviour, but a clearer error about it would be appreciated, instead of

http://jsfiddle.net/skmAm/1/

Changed September 14, 2012 11:39AM UTC by sindresorhus comment:2

_comment0: You can just do $('' + new String('<div>asdf</div>'));1347622838842737
component: unfiledselector
priority: undecidedlow

You can just do

$('' + new String('<div>asdf</div>'));

Changed September 14, 2012 12:25PM UTC by anonymous comment:3

Replying to [comment:2 sindresorhus]:

You can just do
> $('' + new String('<div>asdf</div>'));
> 

Yes, and we did fix this by changing the factory to return string primitives, but the point was that it required some not-so-obvious debugging, because the exceptions were unclear and browser dependent.

Changed September 14, 2012 12:54PM UTC by dmethvin comment:4

_comment0: This isn't something we'd check for. In all places in the documentation we mean the primitive types and not the wrapped objects. A `selector instanceOf String` check won't work for strings created in other frames, and the mere usage of it with some objects causes a [http://ajaxian.com/archives/working-aroung-the-instanceof-memory-leak memory leak] in oldIE.1347627328836694
resolution: → notabug
status: newclosed

This isn't something we'd check for. In all places in the documentation we mean the primitive types and not the wrapped objects. A selector instanceof String check won't work for strings created in other frames, and the mere usage of it with some objects causes a memory leak in oldIE.

Changed September 14, 2012 01:01PM UTC by jaubourg comment:5

We should probably re-asses this for 2.0. Time for a $.isString?

Changed September 14, 2012 01:08PM UTC by Christian Meixner <christian.meixner@bippesbrandao.de> comment:6

And thats why jshint warns about it. Using new String() is not a good idea, because it causes a lot of weired behavior in JS. And you can not rely on the String object, because it will get lost on most operations, e.g:

typeof (new String('a')).toUpperCase() === 'string'

So it's better to not use it in the first place.

So I think this should not be fixed for the sake of jQuery's size and simplicity.

Changed September 14, 2012 01:12PM UTC by dmethvin comment:7

Yeah, this would ripple through the whole code base, not only String but Number and Boolean as well. Sorry it was so hard to debug, but this isn't a jQuery issue as much as a JavaScript one.

Changed September 14, 2012 01:23PM UTC by ilja.everila@liilak.com comment:8

Replying to [comment:7 dmethvin]:

Yeah, this would ripple through the whole code base, not only String but Number and Boolean as well. Sorry it was so hard to debug, but this isn't a jQuery issue as much as a JavaScript one.

Fair enough. The reason for using it is a bit exotic anyway, and just modifying the final factory output to string primitive is a simple and clean solution.

Did not know about the 'instanceof' memoryleak in IE, it just keeps on coming up with new weirdness...