Mparaiso / Pimple.js

Pimple is a Dependency Injection Container for javascript , compatible with all javascript enabled browsers.
26 stars 4 forks source link

pimple and requirejs #3

Closed FlorianGoussin closed 9 years ago

FlorianGoussin commented 10 years ago

Hi,

I have just downloaded your module pimple.js I enjoyed using the other one for PHP, so I was glad to find yours. I want to use it in a requirejs project.

However, I struggle a lot to make it work.

First of all, I had to comment this 3 lines :

//CommonJS
if(module && module.exports){
  module.exports = this.Pimple;
}

because I had this error: Uncaught ReferenceError: module is not defined

and this is how I try to make pimple container work:

define('dependencies', [Validation', 'Pimple'], function(Validation, Pimple) {

  var container = Pimple();
  console.log();
  debugger;

and I've got this error: Uncaught TypeError: undefined is not a function

natorojr commented 9 years ago

I just hit this error as well. There's a simple 1-line fix which I'm about to submit a PR for as a temporary workaround.

--- a/pimple.js
+++ b/pimple.js
@@ -120,7 +120,7 @@
         this.define('pimple',[],function(){return self.Pimple});
     }
     //CommonJS
-    if(module && module.exports){
+    if(typeof exports !== 'undefined' && module && module.exports){
         module.exports = this.Pimple;
     }

However, much better patterns/techniques exit for supporting all environments (AMD, Node.js/CommonJS, and browser) which you'll find being used in many of today's popular libraries.

For example, this is how Backbone and Marionette do it:

(function (root, factory) {
    // Set up Backbone appropriately for the environment. Start with AMD.
    if (typeof define === 'function' && define.amd) {
        define(['underscore', 'jquery', 'exports'], function (_, $, exports) {
            // Export global even in AMD case in case this script is loaded with
            // others that may still expect a global Backbone.
            root.Backbone = factory(root, exports, _, $);
        });
        // Next for Node.js or CommonJS. jQuery may not be needed as a module.
    } else if (typeof exports !== 'undefined') {
        var _ = require('underscore');
        factory(root, exports, _);
        // Finally, as a browser global.
    } else {
        root.Backbone = factory(root, {}, root._, (root.jQuery || root.Zepto || root.ender || root.$));
    }
}(this, function (root, Backbone, _, $) {
    /**
     * build Backbone library
     */
    return Backbone;
}));

I'll submit a separate PR with this proposal. First, I just want to get the bug fixed ASAP and with fewest changes to the code.

FlorianGoussin commented 9 years ago

That's awesome you fix it! Thx! I'll definitely test pimple.js with your fix asap.

Mparaiso commented 9 years ago

merged.