thlorenz / browserify-shim

📩 Makes CommonJS incompatible files browserifyable.
MIT License
933 stars 87 forks source link

Allow a node module to be shimmed by name #107

Closed olsonpm closed 9 years ago

olsonpm commented 9 years ago

according to the following on your recipes page:

" The browser field is used to indicate browserify where the file resides when the main field does not exist in the package package.json, or when the package.json does not exist "

later, you specify:

"browser": { "angular": "./node_modules/angular/angular.js", "angular-cookies": "./node_modules/angular-cookies/angular-cookies.js" },

which is confusing because angular already has a package.config with a main field.

All the documentation I've read on the "browser" field points to angular not needing to be defined, however in my projects - if I don't define it, then angular doesn't work.

bendrucker commented 9 years ago

The section on Angular is outdated since it's available on npm and napa isn't required. Mind creating a quick repo that reproduces your setup so I can take a look?

olsonpm commented 9 years ago

I will try to have a sample repo up tonight.

olsonpm commented 9 years ago

Here's the repo: https://github.com/olsonpm/angularExample.git

in short - if you get rid of:

" "browser": { "angular": "./node_modules/angular/angular.js" }, "

then the app no longer works.

bendrucker commented 9 years ago

Did a little investigating. Right now this setup isn't supported (shimming an otherwise properly structured node module) and you'll need the browser field. However, it should be possible to try to require.resolve the package at some point if it's not mapped in the browser field. The wiki entry on Angular is also fairly out of date so I'll invest some time in updating that.

olsonpm commented 9 years ago

Understood. I appreciate you getting back so quickly.

bendrucker commented 9 years ago

No problem. FYI, the easiest way to diagnose something like this is by enabling diagnostics. You'll see that "angular" is just treated as a path because it's not found in browser.

plumlee commented 9 years ago

EDIT - Self inflicted wound. Turns out it works exactly as it should, once I corrected an error in how I was bundling.

I'm having a very similar problem, but it doesn't appear that I can use require.resolve to get the path I need. (I have to shim these because the browserified bundle is used in an AMD loader, and the UMD wrapper in these breaks things).

Original package.json browser field was:

    "browser": {
        "backbone": "backbone.js",
        "backbone.marionette": "./node_modules/backbone.marionette/lib/backbone.marionette.js",
        "underscore": "./node_modules/underscore/underscore.js"
    },

I was hoping to avoid hardcoding module paths that in the browser field by doing an external shim file like so, using require.resolve to get the full path to the modules:

var shim = {};

var underscorePath = require.resolve('underscore');
var underscoreDeps = {};
underscoreDeps[underscorePath] = '_';

var backbonePath = require.resolve('backbone');
var backboneDeps = {};
backboneDeps[backbonePath] = 'backbone';

var marionettePath = require.resolve('backbone.marionette');
var marionetteDeps = {};
marionetteDeps[marionettePath] = 'backbone';

shim[underscorePath] = {
    'exports': '_'
};

shim[backbonePath] = {
    'depends': underscoreDeps,
    'exports': 'Backbone'
};

shim[marionettePath] = {
    'depends': backboneDeps,
    'exports': 'Backbone'
};

module.exports = shim;

The config gets loaded, build works fine, and when I enable diagnostics things look ok, but the built file is not properly shimmed.

I do have another entry in the browser field for the single library I use that is not loaded by NPM. Removing that doesn't make a difference. I can put up a sample repo if that helps.

bendrucker commented 9 years ago

@thlorenz Just spent a bunch of time with this and I'm not sure it's even possible to get this to work. Getting resolveShims to use node-resolve is easy. The problem is that the transform is never run unless the browser alias is there to map to a relative path. Any ideas?

thlorenz commented 9 years ago

Not sure what you're trying to solve -- lots in this thread, but the above shim thing will not be supported. Just put configs inside package.json should be the advice we give.

Also you shouldn't have to reach into modules like in the example. Instead tell the authors to fix their modules and add a main field to their package.json so you don't have to alias it at all.

olsonpm commented 9 years ago

Yeah I think if you just put a note in the documentation it will be fine. Not that I will think of anything bendrucker hasn't already, but the least I can do is spend a couple hours looking into the code myself. I will make a note to take a look at this over the weekend.

thlorenz commented 9 years ago

@olsonpm do you mind providing that note via a PR? Thanks.

olsonpm commented 9 years ago

Like a PR-to-be? I only really started using github a couple months ago so forgive my misunderstanding. I just meant I'll take a look over this weekend and get back with what I found. If I'm able to find any concrete fixes I'll certainly create a PR with those.

thlorenz commented 9 years ago

PR to add notes to the readme I meant. You can also just fork the repo it and add your notes, we can cherry-pick it into here.

bendrucker commented 9 years ago

Should have clarified. The issue isn't with modules that have an incorrect main entry. It's with things that are published but aren't proper CommonJS modules and still try to attach themselves to window..

olsonpm commented 9 years ago

Ah sure