lorlarz said:
** I re-ran jQuery in JSLint and nothing WHATSOEVER CHANGED IN THE
REPORT ***
(i.e exactly the SAME long list of 'errors' was generated by JSLint.)
I didn't try it myself, but I think that's highly unlikely. I only
clicked on one of the changeset links you posted, and in that set alone,
at least 20-30 JSLint warnings should have been fixed. Did you really
check out the current trunk version of JQuery? HEAD, or whatever it's
called in Git-land (sorry, svn user here).
[snip]
** BIG UPDATE **:
jQuery now passes JSLint 100% but with noted exceptions:
Quoting a recent post from John Resig to the
group
http://forum.jquery.com/developing-jquery-core
Quoting it in-full:
An update: jQuery is now 100% passing JSLint (with a few exceptions,
noted below)
He really led off with that?
and is integrated into the jQuery build process. I've
been making changes to the codebase all day today and it's at a good
state now.
Yes, I'm sure he was anxious to prove me wrong about his willingness to
make changes (and these were very easy ones of course). No need to
thank me. But, as pointed out, this changes very little in terms of the
viability of the script. In fact, it literally changes nothing for
sites that are running on pre-epiphany jQuery.
And it took all day?!
Here is where I integrated JSLint into the jQuery build process:
http://github.com/jquery/jquery/commit/950b5d64a27994db1697eb4e605f5ea48ad8021b
A list of the exceptions that we ignore in our JSLint run can be found
here, with explanation:
http://docs.jquery.com/JQuery_Core_Style_Guidelines#JSLint
"We ignore the following warnings from JSLint:
"Expected an identifier and instead saw 'undefined' (a reserved word)."
- In jQuery we re-declare 'undefined' as we use it internally. Doing so
prevents users from accidentally munging the name (assigning to
undefined and messing up our tests) and also makes the undefined checks
slightly faster."
Messing up their tests?! And I fail to see how using a variable makes
checking for an undefined variable any faster.
""Use '===' to compare with 'null'." and "Use '!==' to compare with
'null'." - Almost universally in jQuery core we use === or !== with the
exception of when we're comparing against null. It's actually quite
useful to do == null or != null as it will pass (or fail) if the value
is either null or undefined. If we want to explicitly check for a null
or undefined value we use === null or === undefined (discussed in depth
below)."
Of course, that was a large percentage of the warnings. And how is it
"quite useful" to write hopelessly ambiguous code? They use that in
lots of places where it shouldn't be expected to match more than one of
those values.
""Expected an assignment or function call and instead saw an
expression." - There are two areas in which we access a property with
seemingly no side effect, those are:
parent.selectedIndex; (or similar) this is due to the fact that WebKit
mis-reports the default selected property of an option, accessing the
parent's selectedIndex property fixes it."
We went over (and over) that. It's a ridiculous incantation that is
easily avoided. Just check the selectedIndex property as it has the
information needed! Don't evaluate it, pray that that has the desired
side effect and then check a different property. If there is one (two
as it is duplicated) "area" that demonstrates the failings of John Resig
to understand basic browser scripting, this is it. He bases decisions
on what he can see happening (today) rather than sound logic. This
leads to scripts that "work" today and fail tomorrow. It's hardly
unexpected that he would choose to ignore this (again).
"Array.prototype.slice.call( document.documentElement.childNodes, 0
)[0].node... - This check simply attempts to determine if, after a slice
on a NodeList, the resulting collection still holds DOM nodes. This is a
feature detection and is wrapped in a try/catch (if the property doesn't
exist it'll throw an exception."
And, that can hardly be what JSLint was complaining about as it isn't
concerned with the code's _logic_ (or host objects).
""Expected a 'break' statement before 'case'." - There is only one
legitimate reason to use switch statements over an object of properties
or a series of if/else statements: passthroughs."
That's meaningless gobbledygook. Who decided that switch statements
without "pass-throughs" were illegitimate? On the contrary,
pass-throughs are exactly what JSLint is (legitimately) complaining
about as they require careful examination to determine whether the lack
of a - hreak - was intended or an oversight. And what the hell is an
"object of properties?"
""We use it inside of Sizzle to optimize our
nly-child selector
checks, reducing code and improving performance."
Whatever.
"Additionally we enable the following two JSLint options by default:
"evil" - This allows code evaluation. We only use it in one place inside
of jQuery: Backwards compatibility support for JSON parsing (we
preferably use the modern JSON.parse method if it's available)."
Evil is right. IIRC, they never did figure out that the Function
constructor is more appropriate for that task.
""forin" - This removes the requirement of doing a hasOwnProperty check
inside of for/in loops. jQuery doesn't support working in an environment
that has manipulated the Object.prototype as it's considered harmful."
Yes, harmful to incompetently written scripts.
The style guide continues:-
"TYPE CHECKS
String: typeof object === "string"
Number: typeof object === "number"
Boolean: typeof object === "boolean"
Object: typeof object === "object""
Cargo cult. There's no reason to use strict comparisons here.
"Function: jQuery.isFunction(object)"
typeof fn == 'function'; // Don't ever use such tests with host objects
"Array: jQuery.isArray(object)"
I'm sure that's the less-than-solid "Miller Device" (another device that
should be designed out of the system, but can't be at this late date).
"Element: object.nodeType
null: object === null
null or undefined: object == null"
That last one is used to death in jQuery, yet it would seem to be (and
should be) rarely needed.
"undefined:
Global Variables: typeof variable === "undefined"
Local Variables: variable === undefined
Properties: object.prop === undefined"
Ridiculously counter-intuitive (and this is because of their undefined
argument trick).
"REGEXP
All RegExp operations should be done using .test() and .exec().
"string".match() is no longer used."
Why? IIRC, they used to use - match - where - test - would be more
appropriate (as I pointed out in numerous reviews), but that is hardly a
style issue as they do two different things.
"STRINGS
Strings should always use double-quotes instead of single-quotes."
Even if they contain double-quotes?
"NODES
..nodeName should always be used in favor of .tagName."
Those are hardly equivalent. This is the usual over-generalization that
leads to would-be programmers shooting their toes off.
".nodeType should be used to determine the classification of a node (not
..nodeName)."
Apples and oranges (again).
"Don't attach expandos to nodes - only attach data using .data()."
....which adds an expando.
When it comes to expandos, it's in for a
penny, in for a pound. They should be avoided at design time, but it's
a bit late for jQuery to change that aspect of their design. And due to
their attr woes, at one point at least, these expandos were being
manifested as custom attributes in IE8. (!)
Admittedly, some modules in My Library (e.g. query) attach an expando to
element nodes, but at least IE is spared (as of late), and IE is the
only major browser known to throw exceptions on such assignments. I
think there is still a check for document.expando === false guarding the
query module, which I can now safely remove. There are also a few other
places where document nodes receive expandos (and are guarded with
similar checks that are no guarantee for non-IE browsers), but those
will be removable once support for frames is relegated to builds that
explicitly request such support (of course, frames-enabled builds will
still require them).
