bundle.require doesn't appear to work in prebundle hook

[nickstenning]

I'm currently in the process of trying to work out why a set of tests have recently started failing, and while I'm not certain that karma-browserify is at fault, I have been able to reduce the testcase to a point where I figured it would be useful to open a discussion here.

Here's the code, in the form of a gist: https://gist.github.com/nickstenning/d70fea282c5897658452

Specifically, I was previously able to call bundle.require(...) in a prebundle hook in order to expose a library to the browserify bundle with a specific name (using browserify's expose option). This no longer appears to work.

Here's the output of karma start karma.conf.js --single-run on the above-linked project:

INFO [karma]: Karma v0.12.28 server started at http://localhost:9876/
INFO [launcher]: Starting browser PhantomJS
ERROR [framework.browserify]: bundle error
ERROR [framework.browserify]: Error: Cannot find module 'foo' from '<homedir>/bundlerequire'
INFO [PhantomJS 1.9.8 (Mac OS X)]: Connected on socket ghbV-cdMt-BQHjVpAzsV with id 46640088
PhantomJS 1.9.8 (Mac OS X) ERROR
  TEST RUN WAS CANCELLED because this file contains some errors:
    /var/folders/8r/_p3vfgtn6d991_lrz2vw1w1m0000gn/T/c1e197606dd046b68f412728a2f0b12dbb763706.browserify
PhantomJS 1.9.8 (Mac OS X): Executed 0 of 0 ERROR (0.01 secs / 0 secs)

[bendrucker]

Did a little investigation and this would have worked with karma-bro < 0.7

[bendrucker]

So the reason this is happening is that the bundle gets reset on rebuilds:

https://github.com/Nikku/karma-browserify/blob/master/lib/bro.js#L208

Immediate fix is to do this in prebundle:

bundle.require('./foo', {expose: 'foo'});
bundle.on('reset', function () {
    bundle.require('./foo', {expose: 'foo'});
});

external among other bundle configuration calls is not reset on reset. Technically running prebundle on every run wouldn't break anything but it would balloon arrays like b._external with long running watchify instances. I think the answer here is to provide an extra hook that runs on every bundle call but I'll wait on @nikku's thoughts before we ship anything.

[nickstenning]

Thanks for digging @bendrucker.

To pre-empt questions about why on earth I'd want or need to do this: I have a project, "foo", which contains some plugins. These plugins depend on "foo-plugintool", an external dependency which in turn depends on "foo". When testing, I'd like that require('foo') to resolve back to the working copy of the project rather than the released version. Hope that makes sense.

[nikku]

Haha, I already had an update for that behavior implemented but reverted it again because no reported a bug on it yet.

The intention during early karma-bro times was to actually be able to configure the whole bundle thing once. Too bad browserify nowadays resets file configurations and so forth.

My suggestion is: We fire a custom event users can listen to that resembles the configuration before actual bundling for the users:

bundle.on('prebundle', function () {
  bundle.require('./foo', {expose: 'foo'});
});

We could then rename the actual method to something like configure or the like to indicate manual configuration of browserify. Prebundle is definitely the wrong name for what it offers today.

[nikku]

Looks like this is absolutely equivalent:

bundle.on('reset', function () {
  bundle.require('./foo', {expose: 'foo'});
});

The initial configuration is not needed as we always reset the bundle prior to any bundling operations.

[bendrucker]

You're right. Honestly I'm inclined to just document this. The gain from having a clearer event name isn't worth creating code that isn't portable (i.e. wouldn't work with regular watchify). configure makes sense but would also mean a major version change.

[nickstenning]

This sounds fine as far as it goes, but it seems that it introduces another related bug. If you do

bundle.on('reset', function () {
  bundle.require('./foo', {expose: 'foo'});
});

then certainly require('foo') works, but oddly require('./foo') no longer does. This is at odds with the usual behaviour of browserify AFAICT. If I have foo.js:

module.exports = 4;

and relative.js:

var foo = require('./foo');
console.log("foo =", foo);

then I can still browserify this using

browserify -r ./foo.js:foo relative.js | node

and the relative require continues to work.

[nikku]

This is related to #52 and a browserify/watchify bug. It looks as if watchify cannot handle files that are required using their relative path names and a global alias at the same time.

As much as I can tell there are two options to fix this, none of them is without pain.

[nikku]

We are going to fix this in karma-browserify by exposing all files via their absolute path.

This is something watchify requires. In order for the above code to work, you should use

bundle.require('abs-path-to-foo', {expose: 'foo'});

[nikku]

Forget what I posted before 😀.

The next version of karma-browserify will remove the prebundle hook in favor of a configure hook.
Users may hook into prebundle actions by listening to the prebundle event.

Example configuration:

{

  files: [ ],
  preprocessors: [ ],

  browserify: {
    configure: function(b) {
      b.on('prebundle', function() {
        b.external('lib/external.js');
      });
    }
  }
}

[nickstenning]

Thank you! 🍰