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

Add support for CommonJS vendors #30

Open
giuliandrimba opened this issue Nov 27, 2013 · 5 comments
Open

Add support for CommonJS vendors #30

giuliandrimba opened this issue Nov 27, 2013 · 5 comments

Comments

@giuliandrimba
Copy link

It isn't possible to load CommonJS vendors that doesn't have the .module extension on it, and is not viable to rename all the CommonJS vendor files to include this extension.

I made a solution, which check if the file has the .module extension on it or the path to the file includes the modules string (folder).

What do you suggest to be the best approach for this?

@sheerun
Copy link

sheerun commented Dec 31, 2013

I think the only reliable solution is to detect two situations described in the README:

  1. You require in files using require():
  2. You export properties using module.exports

@giuliandrimba
Copy link
Author

@sheerun

The require() method, will only find the modules that were wrapped like this:

require.define({'library/name': function(exports, require, module){ /* Your library */ }});

The problem is that only files with the .module extension will be wrapped using the above pattern, thus, making it impossible to wrap vendor libraries in CJS.

If I want to use a vendor like mout, I would have add the .module extension to all the files, which is not a good solution.

As I said, I made a solution, that look up the files that are inside a modules folder, instead of looking up the files based on the .module extension.

This way I can add the CJS vendors to be found by Sprockets-CommonJS, without having to rename the files.

@sheerun
Copy link

sheerun commented Dec 31, 2013

@giuliandrimba I understand your problem. I just say that reliable solution would check if script uses require or module instead of checking if path/filename contains "module" in it.

@giuliandrimba
Copy link
Author

@sheerun This certainly would be the best approach, I made a test-case locally and it worked perfectly, however, I had some problems.

There are scripts that exports only to the window or to Node module object.

Let's take jQuery for instance:
It will export its jQuery object to Node if it finds a module object, otherwise the jQuery object will be exported to the window:

jquery-1.10.2.js

if ( typeof module === "object" && module && 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;

    // Register as a named AMD module, since jQuery can be concatenated with other
    // files that may use define, but not via 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( "jquery", [], function () { return jQuery; } );
    }
}

If we search the files content for module.exports, the jQuery code will be encapsulated, and it won't export itself to the window, which causes all the jQuery plugins to stop working.

Eg:

require.define({'jquery': function(exports, require, module){ 
  /* jQuery Code */ 

  if ( typeof module === "object" && module && typeof module.exports === "object" ) {
    module.exports = jQuery;
  } else {
    /* IT WON'T EXPORT TO WINDOW, THUS, THE PLUGINS WON'T FIND HIM */
    window.jQuery = window.$ = jQuery;
  }
}});

A solution for this, is to export jQuery to the window manually, but I am not sure if this is a good solution, although I think that at this point, all the libraries (jQuery plugins included) should export themselves for different environments.

What do you think?

The code for this change is here (in a different branch)

@sheerun
Copy link

sheerun commented Jan 6, 2014

One more time:

You should check if module.exports is used, but define( is not used elsewhere. That way for jQuery we won't wrap it in define because it does it by itself. For example:

data.to_s.include?('module.exports') && !data.to_s.match?(/[^\.]define\(/)

Of course this Regexp is not enough (for example it fails for some comments and no tabs).

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