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:	unfiled → selector
priority:	undecided → low

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:	new → closed

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]: