Constructor `opts.require` (keep, document, fix?; kill?)

[jmm]

The constructor currently accepts an undocumented opts.require prop. I assume it's a holdover from an earlier version? In any case, can it be either documented and fixed or dropped? Personally I like the idea of having it, if there's not some good reason not to.

It's somewhat broken currently (v9.0.8):

var
  browserify = require('browserify'),
  fs = require('fs'),
  files = [{file: './a'}];

browserify()
  .require(files)
  .bundle()
  .pipe(fs.createWriteStream('./bundle-a1.js').on('finish', function () {
    browserify({require: files})
      .bundle()
      .pipe(fs.createWriteStream('./bundle-a2.js'));
  }));

The first produces a good bundle-a1.js, the second produces TypeError: Arguments to path.resolve must be strings and empty bundle-a2.js.

[terinjokes]

Does this work if you deep clone files before being passed into each require. I don't immediately see a difference between in the code paths.

[jmm]

The difference is that in the first case it loops files and calls .require() again with files[0].file as the first arg, while in the second case it loops files and calls .require() with files[0] as the first arg.

[terinjokes]

It's concerning to me that b.require({file: './a'}); has different behavior than b.require([{file: './a'}]);.

Resolving this difference seems like it would make the browserify({require: [{file: './a'}]}) behavior work properly.

[jmm]

@terinjokes

It's concerning to me that b.require({file: './a'}); has different behavior than b.require([{file: './a'}]);.

I ran into that between the time I posted and read your reply, and I agree. It surprised me.

Resolving this difference seems like it would make the browserify({require: [{file: './a'}]})behavior work properly.

That wouldn't totally resolve it on its own, but you could then resolve it by just calling self.require(opts.require || []) in the constructor. Actually currently the constructor does (opts.require).filter(Boolean), but that's probably better handled in b.require() anyway.

[goodbomb]

Hi, just to chime in quickly, I'm running into this problem right now and it's a real issue for me. I get this error no matter what I try:

Error: path must be a string

[jmm]

@terinjokes Sorry, my mistake -- I don't know why I was thinking that wouldn't work. That ought to work as long as the basedir stuff works out like it should. What I should say is that if that's resolved, the loop should be removed from the constructor and just delegate to b.require() -- no point having redundant looping constructs for this. Do you want to work on this or should I?

[jmm]

@goodbomb You're running into that when passing something as opts.require? What are you passing? What browserify version? You should be able to pass an array of strings, but passing an array of objects won't work currently. If you have an array of objects an alternative would be to construct a browserify instance and pass it to b.require().

[goodbomb]

@jmm I managed to sort it out last night. It would seem I was using it incorrectly. I had upgraded from an earlier version of Browserify and my build broke. I was doing this:

vendorFiles.forEach(function(lib) {
        b.require(lib);
});

But what I actually had to do was this:

var b = browserify({
        require: vendorFiles
});