Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12538 closed bug (notabug)

HTML String instances not handled as string

Reported by: ilja.everila@… 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.

Change History (8)

comment:1 Changed 7 years ago by ilja.everila@…

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/

comment:2 Changed 7 years ago by sindresorhus

Component: unfiledselector
Priority: undecidedlow

You can just do

$('' + new String('<div>asdf</div>'));
Last edited 7 years ago by sindresorhus (previous) (diff)

comment:3 in reply to:  2 Changed 7 years ago by anonymous

Replying to 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.

comment:4 Changed 7 years ago by dmethvin

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.

Last edited 7 years ago by dmethvin (previous) (diff)

comment:5 Changed 7 years ago by jaubourg

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

comment:6 Changed 7 years ago by Christian Meixner <christian.meixner@…>

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.

comment:7 Changed 7 years ago by 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.

comment:8 in reply to:  7 Changed 7 years ago by ilja.everila@…

Replying to 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...

Note: See TracTickets for help on using tickets.