-
-
Notifications
You must be signed in to change notification settings - Fork 460
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
Problem use in requirejs #35
Comments
+1 |
That looks like a RequireJS configuration issue to me. Did you get it working in the end? |
+1 |
@mroderick the jQuery shim expects PubSub to be a global, but since introducing the new UMD wrapper it isn't so you need to check for and require jQuery in AMD and Node environments. e.g.: function createPlugin(jQuery, PubSub) {
// Create the pub sub methods.
}
if (typeof define === 'function' && define.amd){
require(['jquery', 'pubsub'], createPlugin);
} else if (typeof exports === 'object'){
// CommonJS
createPlugin(require('jquery'), require('pubsub'));
} else {
// Browser globals
createPlugin(window.jQuery, window.PubSub);
} |
I've removed the line that calculates the version property for jQuery with 2d171ae as the core property was removed a long time ago. |
You define it as a module for require.js, but the point is to use it as "regular" jQuery plugin.
All plugins I'm using except of jquery.pubsub work this way. |
Here's a simple test:
When using AMD and the JQuery version, the pubsub module binds a useless method to the JQuery object because the globally scoped variables are never created. I don't think is a realistic expectation that every time someone includes PubSub via require.js defines they need to also manually attach the object and define the globals. This can be solved by removing the
|
This change means that it's no longer possible to use PubSubJS without modification if one would like to avoid leaking global vars. Even though it's a small task to remove the global assign from the library when it's not desired – is unconditionally assigning |
I would argue that yes, like JQuery, a globally namespaced object is the most common use case considering the events themselves can be namespaced. Although I do think there's a valid argument for a local object, I believe that should be handled by argument/configuration as it would be the exception, not the norm. |
Just seems like always exposing a global object goes against the purpose of using AMD in the first place. If I want to expose Also, this "feature" was added in a patch release and had some unfortunate side-effects. I can agree however that our usage is probably less common and have no issue with manually patching the lib for our purpose. |
It's weired that when i run rake jquery to get a jquery pubsub version.After that, i use it in requriejs version 2.1.8+ where the config is like
it produced a error said
It seem that the the line 38
``
if (typeof exports === 'object' && module){
module.exports = factory();
The text was updated successfully, but these errors were encountered: