-
Notifications
You must be signed in to change notification settings - Fork 65
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
require js wrapper #8
Comments
Yeah, removing that should be fine. I haven't used this in a require.js context yet so that slipped past. I'll clean that up. Do you use requirejs a lot? Is the more common wrapper pattern to use |
Not sure what's the best practise, this is what was used in underscore 1.2.4 // Export the Underscore object for **Node.js** and **"CommonJS"**, with
// backwards-compatibility for the old `require()` API. If we're not in
// CommonJS, add `_` to the global object.
if (typeof exports !== 'undefined') {
if (typeof module !== 'undefined' && module.exports) {
exports = module.exports = _;
}
exports._ = _;
} else if (typeof define === 'function' && define.amd) {
// Register as a named module with AMD.
define('underscore', function() {
return _;
});
} else {
// Exported as a string, for Closure Compiler "advanced" mode.
root['_'] = _;
} backbone.io isn't defining anything new but just updating Backbone object, so I don't think you need to change anything in your wrapper. Also I noticed the wrapper is defined twice. One for Backbone.Model.prototype.ioBind and one for Backbone.Collection.prototype.ioBind, this is probably due to your build script, but it could probably be avoided what do you think ? |
Did a little research .. the trick with the requireJs wrapper is that you have to pass in the correct backbone instance to extend. If you are not using require/amd, then the Backbone is on the global and all works well. If not, then you need a little bit of extra work. See issue #12. Unfortunately, this would require a bit project reorganizing. Not opposed to it, but was trying to hold out until the next version of backbone, which should be soon. Hopefully that can be done without significant changes to the API. Anyway, fix for this bit will be up later this evening. Head over to #12 if you come across ideas for better amd support. |
Let me know if this gives you further trouble. |
Hello
great work first for this really nice plugin
I tried to drop in the code as is in my amd/require-js app and got an error on
exports = module.exports = ... as module is not defined
do we really need the "module.exports" ? It works fine once I removed it
The text was updated successfully, but these errors were encountered: