Skip to content
Permalink
Browse files

Support CommonJS environments by accentuating the need for a window w…

…ith a document. Fixes #13768.
  • Loading branch information...
timmywil committed Jul 4, 2013
1 parent aee4659 commit 1f67d07c60c37e60052db37fc03d42af482c2d03
Showing with 40 additions and 28 deletions.
  1. +0 −2 Gruntfile.js
  2. +0 −24 src/exports.js
  3. +35 −1 src/intro.js
  4. +5 −1 src/outro.js
@@ -68,8 +68,6 @@ module.exports = function( grunt ) {
{ flag: "offset", src: "src/offset.js", needs: ["css"] },
{ flag: "dimensions", src: "src/dimensions.js", needs: ["css"] },
{ flag: "deprecated", src: "src/deprecated.js" },

"src/exports.js",
"src/outro.js"
]
}

This file was deleted.

@@ -11,7 +11,41 @@
*
* Date: @DATE
*/
(function( window, undefined ) {

(function ( window, factory ) {

if ( typeof module === "object" && typeof module.exports === "object" ) {
// Expose a jQuery-making factory as module.exports in loaders that implement the Node
// module pattern (including browserify).
// This accentuates the need for a real window in the environment
// e.g. var jQuery = require("jquery")(window);
module.exports = function( w ) {

This comment has been minimized.

Copy link
@domenic

domenic Jul 8, 2013

Just an idea... could you do module.exports = typeof window !== "undefined" ? factory(window) : function (w) { if (!w.document) ... }?

That way people who are using CommonJS in the browser can just do require("jquery"), whereas people using it in jsdom do require("jquery")(window).

This comment has been minimized.

Copy link
@timmywil

timmywil Jul 8, 2013

Author Member

That might be a good middle ground. The inconsistency within CommonJS-like environments makes me nervous though. I'd like to hear what @dmethvin thinks. However, CommonJS in the browser is a targeted enough use case that it is worth considering punting that to the user. CommonJS is not for the browser and requires really ugly hacks to make it work.

This comment has been minimized.

Copy link
@domenic

domenic Jul 8, 2013

CommonJS is not for the browser and requires really ugly hacks to make it work.

I strongly disagree, as do many users. (See the many blog posts of people switching from AMD to Browserify, or cujoJS/Curl and Cajon's support for loading CommonJS natively.)

The inconsistency within CommonJS-like environments makes me nervous though.

That is the drawback though, I agree.

This comment has been minimized.

Copy link
@timmywil

timmywil Jul 8, 2013

Author Member

I strongly disagree, as do many users. (See the many blog posts of people switching from AMD to Browserify, or cujoJS/Curl and Cajon's support for loading CommonJS natively.)

The solution in browserify is a bundle that is synchronously loaded, which isn't much of a solution to me (especially during development). curl uses the ugly hacks to which I was referring. I'm not familiar with Cajon.

Regardless, maybe I shouldn't be commenting on my opinions about CommonJS in the browser. That debate probably won't get resolved here.

This comment has been minimized.

Copy link
@domenic

domenic Jul 8, 2013

I think the most important thing to recognize is that personal feelings aren't as important as what many developers are actually using and would like to be supported. I find AMD an ugly hack, but agree that it's good for jQuery to support it as many people use it.

This comment has been minimized.

Copy link
@timmywil

timmywil Jul 8, 2013

Author Member

personal feelings aren't as important as what many developers are actually using

agreed, sorry for getting off track.

This comment has been minimized.

Copy link
@jdalton

jdalton Jul 8, 2013

Member

Just a heads up Component defines module as a function so the type check will bork on it.

w = w || window;
if ( !w.document ) {
throw new Error("jQuery requires a window with a document");
}
return factory( w );
};
} else {
// Execute the factory to produce jQuery
var jQuery = factory( window );

// Register as a named AMD module, since jQuery can be concatenated with other
// files that may use define, but not via a proper concatenation script that
// understands anonymous AMD modules. A named AMD is safest and most robust
// way to register. Lowercase jquery is used because AMD module names are
// derived from file names, and jQuery is normally delivered in a lowercase
// file name. Do this after creating the global so that if an AMD module wants
// to call noConflict to hide this version of jQuery, it will work.
if ( typeof define === "function" && define.amd ) {
define( "jquery", [], function() {
return jQuery;
});
}
}

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

// Can't do this because several apps including ASP.NET trace
// the stack via arguments.caller.callee and Firefox dies if
@@ -1,2 +1,6 @@
// Expose jQuery and $ identifiers, even in
// AMD (#7102#comment:10, https://github.com/jquery/jquery/pull/557)
// and CommonJS for browser emulators (#13566)
return (window.jQuery = window.$ = jQuery);

})( window );
}));

0 comments on commit 1f67d07

Please sign in to comment.
You can’t perform that action at this time.