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

Conflict with QUnit over modules global #13

Closed
andrewchch opened this issue Sep 17, 2012 · 3 comments
Closed

Conflict with QUnit over modules global #13

andrewchch opened this issue Sep 17, 2012 · 3 comments
Assignees

Comments

@andrewchch
Copy link

Hi All,

I just ran into an issue where the "modules" global declared by QUnit (which apparently complies with CommonJS) causes PubSub to think a module loading framework is present and take itself out of the global namespace, causing some of my stuff to break.

See this discussion on the QUnit site:

qunitjs/qunit#190

I notice that KnockoutJS uses the following logic for detecting a module loader:

function(factory) {
// Support three module loading scenarios
if (typeof require === 'function' && typeof exports === 'object' && typeof module === 'object') {
// [1] CommonJS/Node.js
var target = module['exports'] || exports; // module.exports is for Node.js
factory(target);
} else if (typeof define === 'function' && define['amd']) {
// [2] AMD anonymous module
define(['exports'], factory);
} else {
// [3] No module loader (plain <script> tag) - put directly in global namespace
factory(window['ko'] = {});
}
}

Could someone modify the detection code in PubSub accordingly? Much appreciated (I'll have to do this locally in the meantime).

Cheers, Andrew.

@mroderick
Copy link
Owner

Hi Andrew

Thank you for the conflict report!

I've updated PubSubJS in a branch with more specific detection of CommonJS environments.
Could you get it from the branch and verify that it solves the QUnit conflict for you?

Updates in this branch
https://github.com/mroderick/PubSubJS/tree/issue-13

@ghost ghost assigned mroderick Sep 17, 2012
@andrewchch
Copy link
Author

Hi mroderick,

Yes, that does indeed seem to have done the trick :-) Thanks for the quick response.

Cheers, Andrew.

@mroderick
Copy link
Owner

I've baked a new version - v1.3.1, which includes the fix

/Morgan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants