JSLint Reports on last 2 Production Versions of jQuery
in [JavaScript]
|
Prev: FAQ Entry Proposal: What is (function(){ /*...*/ })() ?
Next: FAQ Topic - What is Ajax? (2010-03-01)
From: Stefan Weiss on 2 Mar 2010 13:41 On 02/03/10 19:06, lorlarz wrote: > On Mar 2, 11:27 am, Antony Scriven <adscri...(a)gmail.com> wrote: >> On Mar 2, 3:54 pm, lorlarz <lorl...(a)gmail.com> wrote: >> > /* while ( (chunker.exec(""), m = chunker.exec(soFar)) !== >> null ) { >> > soFar = m[3]; >> > >> > parts.push( m[1] ); >> > >> > if ( m[2] ) { >> > extra = m[3]; >> > break; >> > } >> > } >> > */ > > >> Probably complaining about the comma used as an operator. >> Good advice, I'd say. Yup. That condition made my eyes bleed. > It makes no sense to me that > (chunker.exec(""), m = chunker.exec(soFar)) could be > an expression evaluated as equivalent to null or not > VIA !== and thus it makes no sense to me that > ((chunker.exec(""), m = chunker.exec(soFar)) !== null ) > could be a condition for the while loop, BUT that is > what is happening in the present jQuery code. > > What are they trying to do? What are they doing? How > can it be rewritten so JSLint will accept it?? lolarz, this has already been fixed (you posted the announcement here yourself). The problematic passage is part of Sizzle, and has now been replaced with a do..while loop: // Reset the position of the chunker regexp (start from head) do { chunker.exec(""); m = chunker.exec(soFar); if ( m ) { soFar = m[3]; parts.push( m[1] ); if ( m[2] ) { extra = m[3]; break; } } } while ( m ); That should make it easier to read and understand. http://github.com/jeresig/sizzle/blob/master/sizzle.js#L41 -- stefan
From: lorlarz on 2 Mar 2010 14:08 On Mar 2, 12:41 pm, Stefan Weiss <krewech...(a)gmail.com> wrote: [snip] > > // Reset the position of the chunker regexp (start from head) > do { > chunker.exec(""); > m = chunker.exec(soFar); > > if ( m ) { > soFar = m[3]; > > parts.push( m[1] ); > > if ( m[2] ) { > extra = m[3]; > break; > } > } > } while ( m ); > > That should make it easier to read and understand. > > http://github.com/jeresig/sizzle/blob/master/sizzle.js#L41 > > -- > stefan- Excellent. Thanks stefan. Now I can report the following: I now have to comment out _nothing_: With ALL the things ('big errors') that stopped the JSLint scan of the _present_ development and production versions of jQuery 1.4.2 (which are presently provided via http://jquery.com _and_ will be for at least a month) _replaced_ with for-certain valid replacement: The exact same JSLint error set which I last listed in this thread STILL HOLDS TRUE for this (the current, present) production release of jQuery (that is jQuery 1.4.2) I have every reason to expect that any of the problems of real concern will be fixed in jQuery 1.4.3 , being released in late March or April. This is what John Resig says for the expected date of jquery 1.4.3 (where the concerns will be addressed ), will likely be released. It is still important for people to be aware of the present status of things -- in case things are of concern to them, doing there own assessment and evaluation of the JSLint error (on the full valid 100% scan).
From: lorlarz on 2 Mar 2010 14:20 On Mar 2, 12:40 pm, Richard Cornford <Rich...(a)litotes.demon.co.uk> wrote: > On Mar 2, 6:06 pm, lorlarz wrote: > <snip> > > >>>/* while ( (chunker.exec(""), m = chunker.exec(soFar)) !== > <snip> > > It makes no sense to me that > > (chunker.exec(""), m = chunker.exec(soFar)) could be > > an expression evaluated as equivalent to null or not > > VIA !== > > What does that have to do with anything (that is both your inability > to understand it and comparisons with null)? > > > and thus it makes no sense to me that > > ((chunker.exec(""), m = chunker.exec(soFar)) !== null ) > > could be a condition for the while loop, BUT that is > > what is happening in the present jQuery code. > > The behaviour of comma operators is known and predictable. > > > What are they trying to do? What are they doing? > > Regular expressions retain some state between uses. Specifically, they > retain there - lastIndex - properties. When a regular expression does > not have the global flag set then the - exec - uses the index of the > last match as the - lastIndex - property's value. This allows the use > of the - exec - (or - test -) methods in - while - loops, where the > body of the loop examines each successive match, without the whole > going back and re-matching previous sections of the input. > > This behaviour is very poorly understood by the vast majority of > 'javascript developers' and there are a huge number of mystical > incantations that are employed in order to 'work around' it. This > appears to be an example of one of them as a side effect of calling - > exec - on an empty string will be to have the - lastIndex - property > of the regular expression re-set to zero. > > The most direct (and very probably fastest) way of setting the - > lastIndex - property of a regular expression to zero is to assign zero > to the property. However, in most cases the use of - exec - with a > regular expression with the global flag set is inappropriate to start > with (as - exec - stops at the first successful match, which is what a > non-global flagged regular expression would do) and so the issue can > be fully mitigated by removing the global flag. > > So it may be the case that you 'fix' JQuery by removing the global > flag from chunker's regular expression literal definition and then > delete the - chunker.exec("") -, the comma and the following space > from the expression above. > > > How > > can it be rewritten so JSLint will accept it?? > > Probably as I just described above. > > Richard. I thank you very much for your most learnied input, Richard. stefan provided the fix that Resig provided in last night's official "fix list" -- and I used that instead of commenting out the block to do the JSLint scan of the jQuery 1.4.2 code I just did (and referred to in my last post). I have not fully figured out your response, but I do ask whether you see the Resig fix as the fix that would indeed not only make JSLint happy (which it did), but indeed provide intended (equivalent?) code. It may be of interest to know that the fix from the "fix list" Resig apparently provided _also_ does _not_ appear in last night's "dailies". Apparently the daily are not JSLint fixed. Just for everyones information.
From: Garrett Smith on 2 Mar 2010 15:23 David Mark wrote: > Garrett Smith wrote: >> David Mark wrote: >>> lorlarz wrote: >>>> On Mar 1, 6:56 pm, Stefan Weiss <krewech...(a)gmail.com> wrote: >>>>> On 02/03/10 01:32, lorlarz wrote: >>>>> >> [...] >> >>> "We ignore the following warnings from JSLint: >>> >>> "Expected an identifier and instead saw 'undefined' (a reserved word)." >> That JSLint Error is completely wrong. `undefined` is a property of the >> global object, not a reserved word. It is not an error and should not be >> flagged as an error. > > But using it as an argument name is pretty stupid. > What do you mean by argument name? Variable name? I saw undefined used as a variable name. The reasons were given so that it would be munged and would be faster lookup. [...] >> The short answer to why it should faster is that the identifier >> `undefined` is resolved sooner on the scope chain, rather than looking >> up to the global scope. > > Assuming implementations don't optimize for the undefined identifier in > some way. > undefined is not ReadOnly so I don't know if an optimization would be possible. Recent Mozilla's (Tracemonkey engine) have optimization for global access. Future version will probably include optimized closure access. https://bugzilla.mozilla.org/show_bug.cgi?id=517164 >> [...] >> >>> Evil is right. IIRC, they never did figure out that the Function >>> constructor is more appropriate for that task. >>> >> The function constructor is safer because it does not use global scope. > > I assume you meant that it does use the global scope, whereas eval uses > whatever scope the call resides in. > >> // Use grouping operator to avoid ASI-related error. > > Huh? > ASI = Automatic Semicolon Insertion. If the JSON response begins with a line terminator. For example, the responseText is - "\n { name : "milo" }" - then the function body would be | function()} | return | | { name : "milo" } | } ASI would result in the functionBody being reduced to: | function()} | return; | | { name : "milo" }; | } And the result o f calling that function would be to return `undefined` and not the object. So it's safer to to use grouping operator there. >> var f = new Function("return("+responseText");"); >> >> // later on. >> var responseValue = f(); >> >>> ""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. ;) >>> >> Exactly, IIRC Prototype JS 1.5 or so was modifying Object.prototype. >> They probably got the idea from Crockford, who, actually advocated doing >> that and called scripts that didn't handle it "incompetent". > > And round and round it goes. Regardless, it doesn't make sense to use > unfiltered for-in loops on the Web. > I disagree. I think modifying Object.prototype causes the problem. By not doing that, filters on for-in loops can be avoided. The filters add more clutter and slow the loop down anyway. [...] > I would think that would depend on the implementation. But it is > interesting that they would take the size hit of an extra character for > no reason (they seem to count characters fanatically). > >> The rule I have tried to be consistent with is to use === unless == is >> needed, which is pretty rarely. > > I do the exact opposite. Why use strict if you don't have to? If == is used, the reader has to consider type conversion issues. For example: * if(x == 0) ... * if(x == "object") ... What are acceptable values for x there? What if x is an Object or false? It might be that the caller passing objects or booleans should never happen; that doing so would be a mistake, but then if === is used then there is no question. By using strict equality in those tests, there is no ambiguity. > Regardless of possible minute performance benefits, I think it obscures > the meaning of the code (when I see strict comparisons, I figure there > is a good reason for them). > The strict comparison is only true when the values are the same type. The loose comparisons can be true of the values are of different type. If the reader wants to know what the real intention of that line of code (without looking at other parts), then === tells that. Now with a test inline like:- typeof x == "object" - there is no ambiguity because typeof operator results are always string values. However with: x == "object" Out of context, it is not clear if x is going to be a string or a String. [...] >>> >> My favorite one is Dojo `it instanceof Array || typeof it == "array"`. > > Buffoons. I tried to tell them that was ridiculous, but they just > carped about providing "proof" (and something about ad hominem attacks). "Ridiculous" is a matter of opinion. Fact: It is useless code which is downloaded along with the rest of Dojo. Useless code should be removed. So far nobody has questioned that principle. It shows that they did not know what they were doing. If they've been made aware of the useless code (and apparently you did that), then they should probably want to fix it immediately. I know I would. Do they expect their clients to not recognize it for what it is? Why would anyone who knows better use that, knowing that the general userbase is using it out of ignorance? [...] -- Garrett comp.lang.javascript FAQ: http://jibbering.com/faq/
From: lorlarz on 2 Mar 2010 15:38
On Mar 2, 2:23 pm, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: > David Mark wrote: > > Garrett Smith wrote: [snip] > > >> My favorite one is Dojo `it instanceof Array || typeof it == "array"`. > > > Buffoons. I tried to tell them that was ridiculous, but they just > > carped about providing "proof" (and something about ad hominem attacks).. > > "Ridiculous" is a matter of opinion. Fact: It is useless code which is > downloaded along with the rest of Dojo. Useless code should be removed. > So far nobody has questioned that principle. > > It shows that they did not know what they were doing. > > If they've been made aware of the useless code (and apparently you did > that), then they should probably want to fix it immediately. I know I would. > > Do they expect their clients to not recognize it for what it is? Why > would anyone who knows better use that, knowing that the general > userbase is using it out of ignorance? > > [...] > -- > Garrett > comp.lang.javascript FAQ:http://jibbering.com/faq/ I really do not think it is appropriate to discuss the Dojo library in a jQuery thread. Things are plenty confusing enough without mixing critiques of different libraries in the same thread. |