Skip to content

Commit

Permalink
Remove undefined var declarations
Browse files Browse the repository at this point in the history
  • Loading branch information
@timmywil
timmywil committed Jul 8, 2013
1 parent 1f67d07 commit c93f91e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/intro.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
}

// Pass this, window may not be defined yet
}(this, function ( window, undefined ) {
}(this, function ( window ) {

// Can't do this because several apps including ASP.NET trace
// the stack via arguments.caller.callee and Firefox dies if
Expand Down
2 changes: 1 addition & 1 deletion src/sizzle
Submodule sizzle updated 6 files
+1 −0 AUTHORS.txt
+5 −5 bower.json
+2 −2 dist/sizzle.js
+1 −1 dist/sizzle.min.js
+1 −1 dist/sizzle.min.map
+1 −1 sizzle.js

7 comments on commit c93f91e

@serbanghita
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timmywil why was undefined removed? Shouldn't it protect jQuery from user overwriting the undefined variable? Thanks

@mgol
Copy link
Member

@mgol mgol commented on c93f91e Jul 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serbanghita It's because most projects don't protect against such overwriting so if anyone does that (but.. why?) they're already screwed on so many levels...

@FagnerMartinsBrack
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't undefined turned into an undefined variable to reduce size on minification? Not familiarized with the codebase btw, just watching.

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't saving us that much so the team agreed we'd rather not have it.

@davidmurdoch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any benefit in removing undefined?

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidmurdoch There isn't enough benefit in having it. But yes, one benefit is that it clears up parameters for an eventual jQuery-creating factory (where jQuery could be re-created with a different context and we can document the factory's signature without worrying about this unnecessary undefined variable). We could do var undefined instead at the top of the closure, but like I said, there isn't enough benefit.

@davidmurdoch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder what the GZIP byte diff is for undefined parameter vs var undefined, vs the omitted undefined? If you have them, can you publish them here?

Please sign in to comment.