Bug Tracker

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#7654 closed bug (wontfix)

cleaning warnings in jQuery

Reported by: [email protected] Owned by:
Priority: undecided Milestone: 1.6
Component: unfiled Version: 1.4.4
Keywords: Cc:
Blocked by: Blocking:


I've cleaned the jQuery core code to avoid the warnings that appears in firefox error console (like "function X don't always return a value" or other "undefined properties").

You can access these copies here : http://hq.ariworld.eu/javascript/jquery-1.4.4-clean.js (and http://hq.ariworld.eu/javascript/jquery-1.4.2-clean.js )

There is one warning remaining : "Unexpected token in attribute selector: < ! >." from line 4139 (4159 in my cleaned copy)

There may be some more warnings but I noticed only those appearing in my scripts.

I hope next versions could be fully clean...

Change History (12)

comment:1 Changed 12 years ago by snover

Resolution: wontfix
Status: newclosed

Thanks for the report, but this is not a jQuery bug. Warnings are NOT DANGEROUS and should be ignored. If you can’t deal with ignoring them yourself, please change your error console’s settings so that it only shows errors. We won’t bloat the library to remove superfluous CSS warnings.

comment:2 Changed 12 years ago by [email protected]

I know I can disable warnings and that they are not dangerous... but I always prefer writing scripts that don't produce any warnings rather than disabling these warnings. It's always better. Now if you don't want these corrections, do as you want... It's not difficult to avoid them... it only take some time to clean your file for each new release...

comment:3 Changed 12 years ago by Rick Waldron

I'm just curious... what are we expected to do with those links above? Have you ever looked at the jQuery source code on github.com?


A cursory review of your code reveals nothing to me about what you've cleaned... and I did a control+f for "clean" and found no comments left by you.

comment:4 Changed 12 years ago by ajpiano


his change makes two minor tweaks in property detection for jquery on window, and adds "return null" 40 times.

jQuery passes JSLint, it does not need to pass the... "Don't have any Firebug Warnings" test, especially if it just means adding return null to functions that don't need to return anything.

comment:5 Changed 12 years ago by [email protected]

To ajpiano

I cleaned jQuery to avoid the strict warnings appearing in firefox error console (not firebug). I added "return null" (or sometimes just "null" to existing "return") to avoid "function don't always return a value" warnings. The functions where I added that returned a value yet in a condition or loop. So don't say that I added "return null to function that don't need ...". A "return;" or a "return null;" or a function without "return;" returns the same value (or that could be "undefined"). I used "null" because I saw that you commonly use it in conditions. It don't change anything to the behavior of jQuery... but it's cleaner (... good practice). I also visited JSLint. The validator is stricter than me and display more warnings than me. (I didn't find the "Don't have any Firebug Warnings" option).

To rwaldron,

I just offered to check them and maybe to include these 'cleanings' for the next versions. If you want, I can check the src files form github and clean them and add comments too...

If you don't want my help, I'll just continue cleaning my copies each time a new version comes...

comment:6 Changed 12 years ago by Rick Waldron

The changes you've made are across both jQuery and Sizzle. Also, have you run these against the jQuery test suite?

comment:7 Changed 12 years ago by ajpiano

The "Don't have any Firebug Warnings" test was a made up, figurative test that describes the arbitrary goal of having jQuery not produce any Warnings from the Firefox console. It is not a real option for JSLint, which is the tool that jQuery uses for checking code for integrity and style.

This is not a matter of "not wanting your help," which, if offerred for a different issue, we would gladly accept. It is simply that the "reward" of jQuery not producing warnings in the Firefox console is not worth the bulk of adding these changes to jQuery and Sizzle, which may behave unpredictably as they have not been run against the tests.

Thank you for your time and interest in helping the jQuery project!

Last edited 12 years ago by ajpiano (previous) (diff)

comment:8 Changed 12 years ago by [email protected]

To rwaldron

I didn't use the jQuery test suite (I didn't even know there's one). Are you talking about "http://jsfiddle.net/" ? I only used my test suite... a remake of window.alert function to test and check results... Just tell me where to find that tool and I'll try it.

To ajpiano

I would be happy to help for the more serious issues. Yesterday, I read some of them. What's the procedure?

Now, I've added comments to the modifications that I made in jQuery ( http://hq.ariworld.eu/javascript/jquery-1.4.4-clean+comments.js ). Reading these comments, you'll understand that it couldn't be any bad issue. Give me the test suite to be sure... or don't if a clean code is really not interesting...

comment:9 Changed 12 years ago by Rick Waldron

The jQuery test suite is a collection of 3688 unit tests that jQuery must pass in the browsers I listed above before it can be deemed release worthy.

The tests exist to aid in avoiding major unexpected regressions in the jQuery source (unless the regression is somehow intended ) from one release to another. Imagine, i make one small change - like... returning null when nothing was previously returned. No big deal right? Wrong. Javascript functions ALWAYS return something - if it is not specified then the default behaviour is to return undefined. So what happens when some other piece of internal jQuery code is testing a condition in which it expects undefined to the result of its evaluation... but now you're returning null. This is what happens:

16 Failed tests spanning across the data and css modules. Only 2782 tests were allowed to run because the entire suite dies when it gets to the custom JSON-P tests.

So I removed that test block, which is only 2 tests and those count against the total. Now you're failing 18.

This allowed the rest of the suit to run and finish with 68 failures, plus the removed two for a total of 70 failures across the data, css, ajax, effects and dimensions modules.

This test was run in both Chromium 9.0.595.0 and Firefox 4b7 - in both browsers I ran the unmodified test suite to ensure 100% passing. Tomorrow I will run it against the older versions of IE.

Last edited 12 years ago by Rick Waldron (previous) (diff)

comment:10 Changed 12 years ago by anonymous

You are right... I wrote that before...

(function () { }) () === undefined


(function () { }) () === null


(function () { return; }) () === undefined


(function () { return; }) () === null


(function () { return null; }) () === null


(function () { return null; }) () === null


(function () { return; }) () === undefined


(function () { return; }) () === null


(function () { return undefined; }) () === null


(function () { return null; }) () === null


ok... So let's return undefined...

comment:11 Changed 12 years ago by [email protected]

No... it's not enough... I test now qunit...

comment:12 Changed 12 years ago by Bill

I think the jquery developers need to go through and clean this up. Return null or undefined, whatever it takes in each case to get it right. Seems like it's nothing more than lazy/messy coding, and I don't see any technical reason for not fixing this. Having unnecessary warnings is just bad practice, and not acceptable for many professional users.

Perhaps jquery should make it a standard to address things like this before releasing, just like testing.

Don't take me wrong - I love the product and appreciate all the great work the team has done. But please try to understand the perspective of developers who use this in their professional work and thus can't make excuses or ignore this stuff, as unimportant as it might seem.

Thanks for taking this into consideration, and hopefully the issue can be straightened out without too much trouble.

Note: See TracTickets for help on using tickets.