Re: Javascript Library

On Oct 31, 6:34 pm, John Resig <jere...@xxxxxxxxx> wrote:
Yawwwwnnnnn.... this is, quite possibly, the most inane set of
ramblings set up by someone who has obviously never created a
JavaScript library of any kind, nor has used JavaScript code in any
sort of production environment.

Wrong on all counts.


I'll reply to your alleged critiques to the code below.

To start with, jQuery only supports the following browsers:
- IE 6+
- Firefox 2+
- Opera 9
- Safari 2+

Which is why it is amazing that it has to resort to browser sniffing.
Of course, it likely won't wory very well in say IE8, Opera 10,
Firefox 3, etc.


Anything outside that jurisdiction is not guaranteed to work. But
considering that that selection is something like 99% of all browsers
(and considering that the selection of supported browsers is "good
enough" for Google, Amazon, NBC, IBM, Oracle, etc. etc.) I think it's

Google as an example?! The worst JavaScript developers in history?
Same goes for Amazon. Whatever you support, you shouldn't have to
parse the userAgent string to support it. That was clear 10 years
ago. Where have you been?

safe to say that we've made a good choice.

Hardly. It is woven into many important low-level functions. Here is
a particularly stupid snippet from the get/setAttribute wrapper called
attr:

} else if ( jQuery.browser.msie && name == "style" )

return jQuery.attr( elem.style, "cssText", value );

Do you have any idea how many agents jQuery will identify as IE?

The ones we care about?

So your library is inappropriate for a site on the public Internet?


Do
you have any idea how trivial it is to properly feature detect IE's
broken get/setAttribute functionality?

Pray tell! I must've missed the boolean property that tells me that
it's broken as all get out.

You simply not thinking.


Forget that for the moment, why is this function passing a style
object to itself?

The .attr() function can be a little futzy (it's long due for a
rewrite). For now, it's written this way to keep all attribute/css
special-case logic contained to a single location.

It is a crock any way you slice it.


if ( jQuery.browser.msie && /href|src/.test(name) && !
jQuery.isXMLDoc(elem) )
return elem.getAttribute( name, 2 );

Just what sort of XML document runs script in IE? Certainly not an
XHTML document. Realize that this absurd line of code is evaluated
every time jQuery looks at an element's attribute.

Huh? That gets the interpreted value of an attribute, only in IE, and
only if it isn't within an XML document (and only for the href and src
attributes - which are the ones that have the specific URL-
interpretation problems).

Just like I said. Every time through it is interpreted. You don't
"know" it is IE until you test that useless msie property. And the
question remains: what sort of XML document runs script in IE?


Then there is this gem:

// Certain attributes only work when accessed via the old DOM 0 way
if ( fix[name] ) {
if ( value != undefined ) elem[fix[name]] = value;
return elem[fix[name]];

This demonstrates a stunning lack of understanding of attributes (and
IE's problems with them.) Where's the sniff (or feature detect) for
IE? This IE-specific workaround runs on everything.

This has nothing to do with being IE-specific, or not. Look at the fix
object - it contains a number of properties (both CSS and regular
attribute) that need to be re-written or defer to another attribute.
(For example, this is also used to fix cases where a user
types .attr("readonly") instead of .attr("readOnly")).

And that has everything to do with IE. The fact that you don't
realize that speaks volumes. Do you even understand what the problem
is with get/setAttribute in IE?


Still in the same function:

else if ( value == undefined && jQuery.browser.msie &&
jQuery.nodeName(elem, "form") && (name == "action" || name ==
"method") )
return elem.getAttributeNode(name).nodeValue;

No comment for this magic spell, so there is no telling what it is
trying to do.

The action and method attributes can be overwritten in IE, like so:
<form>
<input type="text" id="action"/>
</form>

Thus, this code handles that case and gets the correct value, special.

Your magic spell is botched. Did it occur to you detect when
attributes are mistakenly treated as properties (hint #2 on that
subject) and then to test getAttributeNode before blindly inferring
its existence from a useless flag?


One thing is certain. This code will run on lots of
non-IE agents.

It only runs on the ones that we care about - but apparently this
mythical, ultra-popular, StrawMan v1.0 is all the rage these days.

What does that mean? There are hundreds of agents in use today that
identify as IE and any Website that uses your script will break them.


if ( value != undefined ) {
if ( name == "type" && jQuery.nodeName(elem,"input") &&
elem.parentNode )
throw "type property can't be changed";
elem.setAttribute( name, value );
}

Another example of confusing attributes with properties. This is also
an IE-specific workaround with no sniff (or feature detect.)

This was done completely intentionally - we don't support dynamically
changing the type attribute on input elements, since it causes an
exception to be thrown in IE (we would rather have a consistent code

Read your code again. It causes an exception under some circumstances
and other problems under other circumstances. Your parentNode test is
incorrect. Try using that code to create a new radio button and you
will see what I mean.

base, across all browsers that we support, than have problems arise
later for users).

Oddly enough, I don't have any trouble setting the type attribute for
input elements, dynamically created or not. Of course, to do this you
have to go back and re-think your strategy concerning finding a
boolean flag that will alert you to IE's botched attribute handling.


And just when you think you have seen every conceivable miscue (all in
a single function mind you.)

if ( name == "opacity" && jQuery.browser.msie ) {
if ( value != undefined ) {
// IE has trouble with opacity if it does not have layout
// Force it by setting the zoom level
elem.zoom = 1;

What in God's name do opacity (and other styles) have to do with
attributes?

See above.

See what above? A tangled mess?


That's enough of that function. Keep in mind that the code therein is
called constantly by jQuery. Unfortunately for those who rely on
jQuery, it is error-prone, confused, inefficient and convoluted.

Error-prone? Doubtful. Where's your test suite-backed attribute/css
manipulation code, I'd love to see it!

I bet you would. My test suite covers quite a few more agents than
yours. And of course, I don't have to worry too much about ones I
don't use or future revisions as I don't employ browser sniffing. See
how that works?


Inefficient, possibly. We've already made a number of optimizations
wherever possible - but many of the checks can't be circumvented.

The whole structure is rotten. It's only 80K. Why not tear it down
and rewrite it?


Convoluted - sure. As I mentioned before, it's due for a re-write.


The whole thing, right?

The
design (and documentation) is of such poor quality that it is an
assault on the senses of anyone unfortunate enough to glance at it.

Sigh. Where's your ultra-documented, totally-awesome, perfect-in-every-
way JavaScript library? That's what I thought - you don't have one,
nor does one exist.

That is the Matt Kruse school of baiting. I am not going to help you
rewrite your library any more than I already have. Sorry.


And speaking of opacity. In the curCSS function:

if (prop == "opacity" && jQuery.browser.msie) {
ret = jQuery.attr(elem.style, "opacity");
return ret == "" ? "1" : ret;

}

So any agent that identifies as IE and will be re-routed to the attr
function, which has nothing to do with style, but does contain more
sniffing and some IE-specific DirectX twiddling, which will have no
(positive) effect on non-IE agents.

See above - it works for everything that we support. We make no
illusions as to otherwise.

But many delusions as to why you need to support only five browsers
and why you need to sniff the userAgent string to do it.


It would seem that querying style rules would be almost as critical to
querying attributes in a library like jQuery. Yet, in this same
function you find this gem:

// A helper method for determining if an element's values are broken
function color(a){
if ( !jQuery.browser.safari )
return false;

var ret = document.defaultView.getComputedStyle(a,null);
return !ret || ret.getPropertyValue("color") == "";

}

This is the author's idea of a feature detect for getComputedStyle.
Why Safari-like browsers are excluded is a mystery. After that
inauspicious start, it blunders into the defaultView, getComputedStyle
and getPropertyValue properties with the impunity of somebody who
hasn't got a clue as to what they are doing.

Wow. WowWowWow. Spoken like a true babe! It's damn simple to detect it

You come off as an idiot.

in all browsers but Safari. In Safari 2, in an element that is
display: none, getComputedStyle() returns null. In Safari 3, it

That problem is not limited to Safari.

returns a style object, but within with all values return "" (which

Not in Windows Safari 3. Regardless, I was referring to something
else entirely. It looks like you do have a feature detect for
getComputedStyle, but the section of code is so convoluted that I
missed it. Adjusted score is about negative a million plus one.

Also, the handling of this issue is typically inept. You should never
try to query the computed style of an element that isn't part of the
layout. Barring (!important) user style *** overrides, calling
programs should be aware when an element is part of the layout and
when it is not. All of those hoops you are going through to display
parentNodes (with an arbitrary rule no less) is a waste of time.
You've got much bigger and more important problems.

may, or may not, be a valid return value - it's impossible to tell!).
Thus, you have to check for the existence of the computed color value
(which will always exist) and if it's empty, then we're inside a
corrupted element.

It goes on and on. From the clean function:

if ( jQuery.browser.msie ) { // String was a <table>, *may* have
spurious <tbody>
if ( !s.indexOf("<table") && s.indexOf("<tbody") < 0 )
tb = div.firstChild && div.firstChild.childNodes;

div *may* have a firstChild property.

Yes? Your point? IE can, sometimes, not auto-generate a <tbody>

You missed the point completely. The general theme is feature
detection (and the lack thereof.) I didn't spell everything out for
you as this was not a support request ticket. Look a couple of lines
down from that and find:

for ( var n = tb.length-1; n >= 0 ; --n )

This assumes that tb was actually set to something. What if the agent
had no firstChild property? Certainly many agents that identify as IE
fall into that category. Isn't this script supposed to make it easier
to build a Website? Seems odd that it is completely unsuitable for
the Internet.

wrapper (when there's no contents to the table). This verifies when a
<table></table> is passed in makes sure that a tbody is constructed.
Again, everything here is backed by a test suite.

But even if it aces your limited tests, it is still unsuitable for
building Web applications. So what is it good for?


From the same function:

// IE can't serialize <link> and <script> tags normally
jQuery.browser.msie && [1, "div<div>", "</div>"]

You have to keep in mind while reading this stuff that
jQuery.browser.msie is meaningless on the Web. It isn't of much use
on an Intranet either, unless you use the same version and
configuration of IE forever.

Huh? Show me a drastically improved version of IE and I'll show you an
updated copy of jQuery.

What does that mean? If you had written it right to begin with, you
wouldn't have to go back and rewrite it every time a new version is
released. And the other browsers you support are updated constantly.
Any way you slice it, people that rely on your code are actually
relying on you. That is a scary thought as you clearly don't know the
first thing about JavaScript, let alone browser scripting.


From merge:

// Also, we need to make sure that the correct elements are being
returned
// (IE returns comment nodes in a '*' query)
if ( jQuery.browser.msie ) {
for ( var i = 0; second[i]; i++ )
if ( second[i].nodeType != 8 )
first.push(second[i]);
} else

Same problem different function. What if some agent that doesn't
identify as IE has the same problem?

These browsers that you keep talking about must be amazing! They're,
quite literally, everywhere! At every turn there's a browser that we
support that also claims to be IE, but it isn't IE! What have we been
doing wrong all this time? How could we have been so naive?

You read it backwards apparently. The point is that the problem may
exist in browsers that are NOT IE. Why make stupid assumptions? Here
is that portion of the code:

if ( jQuery.browser.msie ) {
for ( var i = 0; second[i]; i++ )
if ( second[i].nodeType != 8 )
first.push(second[i]);
} else
for ( var i = 0; second[i]; i++ )
first.push(second[i]);

What's wrong with that picture? I don't mean the missing brackets
that make it nearly impossible to follow and easy to foul up during
the inevitable maintenance.


Also note how hard it is to
follow with all of the missing brackets.

*shrug*

That captures the essence of your code and comments perfectly.


More madness:

var styleFloat = jQuery.browser.msie ? "styleFloat" : "cssFloat";

Obviously you should just check (or set) them both. What happens if
IE8 switches gears on this? And what of an IE-spoofing agent that
doesn't use "styleFloat?"

The same argument, over and over, is getting incredibly old. Where's
your ultra-popular library that is 100% future proof?

The difference between you and me is that I don't give away code for
beer money donations. Get it? And I gave you the answer to that
one. So why not get rid of at least that one browser sniff. Do you
really need to peek at my style functions to do this?


Considering that IE is going to be backwards compatible for, most
likely, many many years to come, I'm not worried. When the year 2031
arrives, jQuery can, and will adapt.

It is absurd to think the jQuery will be around even a few more years,
let alone decades.


// Check to see if the W3C box model is being used
boxModel: !jQuery.browser.msie || document.compatMode == "CSS1Compat",

That could have far-reaching implications. A quick search found it in
an attempt to measure the viewport:

return this[0] == window ?
jQuery.browser.safari && self["inner" + name] || jQuery.boxModel &&
Math.max(document.documentElement["client" + name],
document.body["client" + name]) ||
document.body["client" + name] :
this[0] == document ?
Math.max( document.body["scroll" + name], document.body["offset" +
name] ) :
h == undefined ?( this.length ? jQuery.css( this[0], n ) : null ) :
this.css( n, h.constructor == String ? h : h + "px" );

I characterized this as a "million monkey" project earlier and I want
to take a moment to apologize to monkeys everywhere. I think even a
single monkey could write a better solution to this very common
problem. Certainly there are many better solutions floating around on
the Internet.

Certainly! They are, literally, everywhere! I mean, I can't go
anywhere without tripping over a test-suite-backed, cross browser,
quirksmode-handling, browser width and height detection script.

Idiot. I posted one here not three weeks ago. It was tested on
virtually everything, all the way back to IE5/NS6 and you can be sure
that the word "userAgent" does not appear in it. It is a good idea to
read the newsgroup and its FAQ (to quote another regular's sig.)


Your "arguments" consist of nothing but assumptions dressed up as
criticism.

I don't have to assume. I have dealt with all of the same problems
that you are stumbling over.


And boxmodel can also be observed in the positioning code:

// IE adds the HTML element's border, by default it is medium which is
2px
// IE 6 and IE 7 quirks mode the border width is overwritable by the
following css html { border: 0; }
// IE 7 standards mode, the border is always 2px
if ( msie ) {
var border = jQuery("html").css("borderWidth");
border = (border == "medium" || jQuery.boxModel &&
parseInt(version) >= 7) && 2 || border;
add( -border, -border );
}
There are so many misconceptions here it is hard to know where to
begin. Suffice to say that the clientLeft/Top properties hold the
needed information in IE.

Yes, we are well aware of the clientLeft/Top properties -
unfortunately there are edge cases where they are not sufficient
(especially when moving between standards and quirksmode).

Gibberish. If you had any idea what you were doing, you wouldn't make
patently false comments like:

"IE 7 standards mode, the border is always 2px"

Do you understand that in quirks mode there is no HTML element present
in the layout. That 2px (which is not always 2px) represents part of
the chrome. It has nothing to do with the border style of the HTML
element. You can give the HTML element a border of 200px and it will
have no affect on the layout and certainly no affect on positioning.
In quirks mode it is the BODY border that gets subtracted from the
coordinates returned by getBoundingClientRect. No magic spells are
needed when you understand what is going on. Edge cases indeed.

It is pretty clear that you don't know what you are doing, which is
why you shouldn't be doing it. Not in public anyway. The fact that
you are also snotty towards criticism comes as no surprise. Most
incompetents have the same attitude.


You're definitely proving that it's much easier to make off-the-cuff
criticisms than to actually provide working, tested, cross-browser
solutions. All of which we've done, and supported, for almost 2 years
now.

Working? Cross-browser? Hardly. If you tested, then why are you
still clueless about simple things like the effect of borders on
positioning? Try turning on HTML borders in any other of your
supported browsers and see what happens to your positioning tests.
Same goes for margins. BODY borders are more prevalent and will also
cause problems in some cases. From the above comment about "edge
cases" for "HTML borders" in quirks mode, it appears you never tested
those at all.


In the same function:

// Mozilla and Safari > 2 does not include the border on offset
parents
// However Mozilla adds the border for table cells
if ( mozilla && /^t[d|h]$/i.test(parent.tagName) || !safari2 )
border( offsetParent );

Apparently, in the jQuery universe there is only Opera, Mozilla,
Safari and IE. IE never gets here. Opera (and Safari <= 2 if you
believe the comments) adds borders when it shouldn't. So this logic
excludes everything the author has ever heard of that isn't Opera.

Right. See above.

But you still don't see why the logic and comments are senseless?
Opera (and Safari 2) are the exceptions, not the rule. The "borders-
included-in-offsets" anomaly is easy enough to feature test.
Coincidentally, I have posted examples of it in the past. They
weren't posts about jQuery, so I guess you didn't read them. Every
time a new version of Opera or Mozilla (or whatever) comes out, you've
got a lot of re-testing and (possibly re-working) to do, which can
easily introduce new bugs, which leads to more re-testing, etc. Then
the dialog pops up telling you that Firefox has a new version and do
you still think this is a good strategy? Since all of your testing is
based on browser names, what exactly will you do when Opera fixes
their border problem? Add another flag based on the version number?
You mentioned something about 2035 (or some ridiculously far off year)
earlier. How long will jQuery's browser sniffing branches be by
then? It is a rhetorical question. Obviously it will have long since
gone the way of the dodo.


Speaking of Mozilla. Does this look like a good indication of Mozilla-
based browsers?

mozilla: /mozilla/.test(userAgent) && !/(compatible|
webkit)/.test(userAgent)

Why not call that the everything-under-the-sun property?

Because it's not? It captures everything that we care about.

Does it indicate Mozilla? IE has spoofed Mozilla in its userAgent
string since the beginning. So is IE Mozilla? Or perhaps in the
various branches it never checks Mozilla unless it has branched away
from IE? Does any of this sound like good coding practice to you?
Variable that have meaningless names, strange twisting branches based
on a string of characters that has no relation to the terrain, adding
more branches every time a new browser version comes out, etc.? Why
would anybody do that? A better question is why would anybody rely on
somebody who does it (especially when the person refuses to listen to
the advice of those who know better.)


Getting back to border issue, in the mozilla-and-newer-Safari-only
(and oddly named) border function:

add( jQuery.css(elem, "borderLeftWidth"), jQuery.css(elem,
"borderTopWidth") );

What an outrage. The author apparently never heard of clientLeft/Top.

See above.

LOL. I think you need to see above. More "edge cases?" You have got
to be kidding. To find the border of an element, you start with
clientLeft/Top. Computing styles is the last resort for browsers that
do not support clientLeft/Top. Considering how outrageously
inefficient your style wrappers are, you would be well advised to use
the clientLeft/Top properties when they are available.


Backing up to where boxModel was first spotted, this is deja vu:

styleFloat: jQuery.browser.msie ? "styleFloat" : "cssFloat",

Yes, this logic is repeated just three lines below the first instance.

This was already fixed in SVN:http://dev.jquery.com/browser/trunk/jquery/src/core.js#L1180


It should have been fixed before it was released.

Moving on. In the ready function:

if ( jQuery.browser.mozilla || jQuery.browser.opera )
document.removeEventListener( "DOMContentLoaded", jQuery.ready,
false );

So if it is virtually any agent, assume there is a document object (a
fair assumption) and assume that it has a removeEventListener method
(an outrageous assumption.) Why the event listener needs to be
removed at all is another interesting question. The comments
mentioned memory leaks.

Man, why am I not using this StrawMan browser? It is, apparently, used
everywhere. Once again, if it's not in the 99% of all browsers that we
support, we don't care.

You are deluded. You don't support anywhere near 99% of all
browsers. And whether you care is not the point. Who was talking to
you?


We remove the event to be tidy - if we don't need to keep it bound,
then there's no reason to waste any perfectly-good memory on it. I
don't understand why this is an issue for you. Do you prefer that
libraries keep everything bound indefinitely? Do you want them to have
high overhead and run slow?

GC is automatic. Do you understand that a memory leak is an exception
to GC? Your comment indicated that you thought the event might leak.
What do you base that suspicion on? Do you automatically remove every
listener every time the page unloads, just in case it might cause a
memory leak?


Here is the flip-side of this in bindReady:

// If Mozilla is used
if ( jQuery.browser.mozilla || jQuery.browser.opera )
// Use the handy event callback
document.addEventListener( "DOMContentLoaded", jQuery.ready,
false );

So two of the four browsers that the author knows of are known to
support DOMContentLoaded. Of course, it wouldn't hurt to add this
listener for everything. Come to think of it, he did (unknowingly)
add it for virtually everything.

Except for half the browsers - Safari and IE. Which is not "virtually
everything".

Not IE? Are you sure? Last I checked, IE spoofed Mozilla. I think
it was about ten years ago. I doubt it has changed.


You've got to love the comments. "Use the handy event callback" may
be the least informative one yet. Here's another interesting one from
makeArray:

// Need to use typeof to fight Safari childNodes crashes
if ( typeof a != "array" )
for ( var i = 0, al = a.length; i < al; i++ )
r.push( a[i] );
else
r = a.slice( 0 );

And why would you pass an array to a "makeArray" function anyway?

.makeArray() is designed to take any array-like object and return a
clone of itself. Thus this will work for both NodeLists and regular
arrays (for example). Note the key word there: clone - it doesn't
return the original (hence it's only used for when an array needs to
be cloned).

You should try coming up with logical names for your functions. You
notice you how had to explain what that function did?

Regardless, you can't see what is wrong with this line?

if ( typeof a != "array" )

What (in any JavaScript implementation) will evaulate that to false?
What is an array in JavaScript?


This reminds me of the function test code, which was blistered in
another recent "jQuery is a joke" thread.

I assume that you're referring to jQuery.isFunction(). If you have a
comparable method that's able to pass all of these tests in all modern
browsers, let me know! I'll be happy to take a look.

We just went over this a few weeks ago. Search the group before
posting. It might help you avoid shooting yourself in the foot next
time.


http://dev.jquery.com/browser/trunk/jquery/test/unit/core.js#L62


Test your own code.

Enough is enough. If you use jQuery after reading this, you are
deserve everything you get.

Apparently, a thoroughly tested and comprehensively written JavaScript
library - who knew?

Nobody knew that, though some thought they did.


Asked and answered? I still want to know what reak-world you are
from.

Probably the same one that Google, Amazon, IBM, AOL, etc. are from.

Again with the Google and Amazon (and AOL?!) nonsense. Is that
supposed to be funny?


Really? Define packed. Minified it is roughly 50K. GZIP doesn't
enter into it (unless you can't comprehend why it is silly to compare
file sizes after compression.) How small would a 20K library be after
compression? Regardless, you shouldn't manually compress JavaScript
files.

Run through Dean Edwards' Packer script. jQuery is ~14K minified +
gzipped.

Which would imply that packer does a worse job than GZIP, so it should
not be used. And of course, GZIP doesn't factor into page weight
comparisons. The server might GZIP it, it might not. All things
equal, it is a 50K script minified.


[snip non-discussion]

So besides .attr() needing a rewrite (I fully admit that) - I see
absolutely no valid criticism of the library - and if anything, a

You are blind. Others certainly did. Even the jQuery fanboy Matt
Kruse agreed with enough of it to open up tickets on your site. I
wondered where he got the idea that you knew better about some of this
stuff as I didn't realize you were nuts enough to actually post a
retort here. When I saw that Matt "re-printed" one of his earlier
responses to you, I couldn't believe my eyes. I knew it would be a
bunch of clueless nonsense, but this exceeded my expectations by far.

gross misunderstanding of common cross browser issues and of how test
driven development works.

Let me know when you release your library, as I'd love to take a look
and see what I've been missing.

I bet you would like to crib from me. And who said I was releasing a
public general-purpose JavaScript library? Seems to me I said just
recently that I would never do that (much to the chagrin of several
people in this thread.) Apparently you don't read before you post.

.