Skip to content

Commit

Permalink
Made jQuery.type more consistent with host objects.
Browse files Browse the repository at this point in the history
  • Loading branch information
rkatic authored and jeresig committed Sep 23, 2010
1 parent 7367b52 commit 484cc6e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
12 changes: 10 additions & 2 deletions src/core.js
Expand Up @@ -69,7 +69,10 @@ var jQuery = function( selector, context ) {
push = Array.prototype.push,
slice = Array.prototype.slice,
trim = String.prototype.trim,
indexOf = Array.prototype.indexOf;
indexOf = Array.prototype.indexOf,

// [[Class]] -> type pairs
class2type = {};

jQuery.fn = jQuery.prototype = {
init: function( selector, context ) {
Expand Down Expand Up @@ -487,7 +490,7 @@ jQuery.extend({
type: function( obj ) {
return obj == null ?
String( obj ) :
toString.call(obj).slice(8, -1).toLowerCase();
class2type[ toString.call(obj) ] || "object";
},

isPlainObject: function( obj ) {
Expand Down Expand Up @@ -799,6 +802,11 @@ jQuery.extend({
browser: {}
});

// Populate the class2type map

This comment has been minimized.

Copy link
@rwaldron

rwaldron Oct 18, 2010

Member

Take a look at this: http://jsperf.com/declared-literal-vs-string-split-2

I've previously noted that literal syntax is twice as fast as the string split syntax.

This comment has been minimized.

Copy link
@jdalton

jdalton Oct 18, 2010

Member

This operation is done once, there will be zero performance impact ether way.

This comment has been minimized.

Copy link
@eligrey

eligrey Oct 18, 2010

The filesize benefits of a simple token list separated only by spaces greatly outweigh the infinitesimal speed boost you'll get with array literal syntax, as this is only run once.

This comment has been minimized.

Copy link
@rkatic

rkatic Oct 18, 2010

Author Contributor

Also maybe space separated words are something more easy to read/write. As well this few lines of code would be more readable if only space separated :)

This comment has been minimized.

Copy link
@rwaldron

rwaldron Oct 18, 2010

Member

I'm on board with all of these points.

jQuery.each("Boolean Number String Function Array Date RegExp Object".split(" "), function(i, name) {

This comment has been minimized.

Copy link
@leeoniya

leeoniya Oct 15, 2010

perhaps add "Error" to the list?
maybe even "Math" - not to sure about this one

http://www.herongyang.com/JavaScript/Built-In-Object-Global-Properties-and-Functions.html

This comment has been minimized.

Copy link
@jeresig

jeresig Oct 15, 2010

Member

Error might be useful - Math, probably less so :)

This comment has been minimized.

Copy link
@eligrey

eligrey Oct 16, 2010

How about XML (E4X) too?

This comment has been minimized.

Copy link
@jeresig

jeresig Oct 16, 2010

Member

@eli: We've been opting not to go down the path of dealing with DOM things (mostly because they're quite inconsistent across platforms - and come to think of it, so are errors).

This comment has been minimized.

Copy link
@eligrey

eligrey Oct 16, 2010

XML isn't part of the DOM. It's a core data type of ECMA-357 (implemented by Mozilla and Adobe) that also returns "xml" by typeof.

This comment has been minimized.

Copy link
@jeresig

jeresig Oct 16, 2010

Member

@eli: Sure, but again, we like working with things that are consistent across browsers - not much point in adding overhead to something that is only in one browser (as you pointed out).

This comment has been minimized.

Copy link
@eligrey

eligrey Oct 18, 2010

Edit: Oops, replied to the wrong line thread.

This comment has been minimized.

Copy link
@jaubourg

jaubourg Oct 18, 2010

Member

It may be nitpicky but wouldn't it be easier to see what is and what is not in the list if types were ordered alphabetically?

class2type[ "[object " + name + "]" ] = name.toLowerCase();
});

browserMatch = jQuery.uaMatch( userAgent );
if ( browserMatch.browser ) {
jQuery.browser[ browserMatch.browser ] = true;
Expand Down
6 changes: 5 additions & 1 deletion test/unit/core.js
Expand Up @@ -219,7 +219,7 @@ test("trim", function() {
});

test("type", function() {
expect(18);
expect(22);

equals( jQuery.type(null), "null", "null" );
equals( jQuery.type(undefined), "undefined", "undefined" );
Expand All @@ -239,6 +239,10 @@ test("type", function() {
equals( jQuery.type(new Date()), "date", "Date" );
equals( jQuery.type(new Function("return;")), "function", "Function" );
equals( jQuery.type(function(){}), "function", "Function" );
equals( jQuery.type(window), "object", "Window" );
equals( jQuery.type(document.body), "object", "Element" );
equals( jQuery.type(document.createTextNode("foo")), "object", "TextNode" );
equals( jQuery.type(document.getElementsByTagName("*")), "object", "NodeList" );
});

test("isPlainObject", function() {
Expand Down

0 comments on commit 484cc6e

Please sign in to comment.