Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Node-like module loaders #1103

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 24 additions & 16 deletions src/exports.js
@@ -1,18 +1,26 @@
// Expose jQuery to the global object
window.jQuery = window.$ = jQuery;
if ( typeof module === "object" && typeof module.exports === "object" ) {
// Expose jQuery as module.exports in loaders that implement the Node
// module pattern (including browserify). Do not create the global, since
// the user will be storing it themselves locally, and globals are frowned
// upon in the Node module world.
module.exports = jQuery;
} else {
// Otherwise expose jQuery to the global object as usual
window.jQuery = window.$ = jQuery;

// Expose jQuery as an AMD module, but only for AMD loaders that
// understand the issues with loading multiple versions of jQuery
// in a page that all might call define(). The loader will indicate
// they have special allowances for multiple jQuery versions by
// specifying define.amd.jQuery = true. Register as a named module,
// since jQuery can be concatenated with other files that may use define,
// but not use 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.amd.jQuery ) {
define( "jquery", [], function () { return jQuery; } );
// Expose jQuery as an AMD module, but only for AMD loaders that
// understand the issues with loading multiple versions of jQuery
// in a page that all might call define(). The loader will indicate
// they have special allowances for multiple jQuery versions by
// specifying define.amd.jQuery = true. Register as a named module,
// since jQuery can be concatenated with other files that may use define,
// but not use 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.amd.jQuery ) {
define( "jquery", [], function () { return jQuery; } );
}
}
2 changes: 1 addition & 1 deletion src/outro.js
@@ -1,2 +1,2 @@

})( window );
})( this );
Copy link
Member

Choose a reason for hiding this comment

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

If "use strict" is in effect, this isn't set to the global object is it? Also, window and the global object aren't always identical, I forget the specifics but IE 6/7/8 may have issues.

Copy link

Choose a reason for hiding this comment

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

That would only be a concern if jQuery source was naively concattenated with other files that included bare "use strict"; not sure how much of a concern that is. If so, then I believe (false || eval)("this") will do the trick.

Copy link
Member

Choose a reason for hiding this comment

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

If someone were using this under node, would they want what jQuery considers to be the window object to be the global object, or would it be better for them to create a global window object and pass that in with whatever methods were needed? It seems like the former requires more pollution of the global namespace.

Copy link

Choose a reason for hiding this comment

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

That's a good point. In my experience using jQuery in Node directly (as opposed to in browserify), I always load it in a sandbox which has the global equal to a jsdom window. So it doesn't make any difference in that case. And it doesn't make any difference in the browserify case either, where they're identical. /shrug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Node, this is the exports object (for reasons of historical weirdness). But if we're not setting a global reference, then it doesn't matter.

The surest "global getter" that I've ever seen is (function () { return this; }).call(null). You could do something like (function () { return this; }).call(null).window so that it's empty if there's no global window defined, or the global window in any web browser.

Copy link
Member

Choose a reason for hiding this comment

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

Can we rely on this being in the global scope? What if a site concatenates this and other modules in one request and wraps it in a loader closure?

Copy link
Member

Choose a reason for hiding this comment

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

Definite possibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Krinkle Would you prefer doing something like typeof window === 'undefined' ? this : window?

Copy link
Member

Choose a reason for hiding this comment

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

@isaacs Hm.. maybe, but why can't it be just window. We only use it in a browser environment, right? In Node for example we set exports, or are we (ab)using the historical this being the exports object in Node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Krinkle Well, no, that's the point. When run in Node, jquery requires a window to already be defined, or it throws an undefined reference at this point. It looks like you really do want it to be the global though, not some random object. (There's window.String stuff, it looks like.)