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

watchify loses expose binding on update #72

Closed
neocoder opened this issue Jul 30, 2014 · 14 comments
Closed

watchify loses expose binding on update #72

neocoder opened this issue Jul 30, 2014 · 14 comments
Labels

Comments

@neocoder
Copy link

Hi, I'm trying to use watchify with karma but watchify doesn't expose require after the update. After the first bundle() call everything is ok, require is exposed to the global scope, but after update - it's gone.

Here's one of watchify modified tests

var test = require('tape');
var watchify = require('../');
var browserify = require('browserify');
var vm = require('vm');

var fs = require('fs');
var path = require('path');
var mkdirp = require('mkdirp');

var os = require('os');
var tmpdir = path.join((os.tmpdir || os.tmpDir)(), 'watchify-' + Math.random());

mkdirp.sync(tmpdir);

var file = path.join(tmpdir, 'main2.js');
fs.writeFileSync(file, 'console.log(555); module.exports = function(){ console.log(123); };');


test('require api', function (t) {
    t.plan(5);
    var w = watchify(browserify(file, watchify.args));
        w.require(file, { expose: 'EXPOSE_TEST' });

    w.on('update', function () {
        w.bundle(function (err, src) {
            t.ifError(err);

            src+='require("EXPOSE_TEST")();';

            t.equal(run(src), '777\n888\n');
            w.close();
        });
    });
    w.bundle(function (err, src) {
        t.ifError(err);

        src+='require("EXPOSE_TEST")();';    

        t.equal(run(src), '555\n123\n');

        setTimeout(function () {
            fs.writeFile(file, 'console.log(777); module.exports = function(){ console.log(888); };', function (err) {
                t.ifError(err);
            });
        }, 1000);
    });
});

function run (src) {
    var output = '';
    function log (msg) { output += msg + '\n' }
    vm.runInNewContext(src, { console: { log: log } });
    return output;
}
@neocoder
Copy link
Author

Here's a temporary patch for the browserify index.js in case somebody experienced the same issue.
@substack Could you please take a look and create a proper fix? The problem is somewhere in the module-deps. There must be something wrong with the packageCache as removing it fixes the issue.

commit edb95edff13c70c2bc4b74b295037a847676eb32
Author: Alex Ladyga <neocoder@gmail.com>
Date:   Thu Jul 31 14:09:15 2014 +0300

    Temporary fix for the require+expose during multiple bundle() calls

diff --git a/index.js b/index.js
index 5766c0d..3c4e869 100644
--- a/index.js
+++ b/index.js
@@ -95,7 +95,13 @@ Browserify.prototype.require = function (file, opts) {
                 self._expose[id] = filename;
             }
             if (!opts.entry && self._options.exports === undefined) {
+                // during the first bundle() call
+                // pipleline and thus _bpack is initiated before require call
+                // so this works...
                 self._bpack.hasExports = true;
+                // but on subsequent bundle() calls require() is not called anymore and we
+                // have to modify global options object to retain the hasExports flag.
+                self._options.hasExports = true;
             }
             var rec = {
                 source: buf.toString('utf8'),
@@ -131,6 +137,7 @@ Browserify.prototype.require = function (file, opts) {

     if (!row.entry && this._options.exports === undefined) {
         this._bpack.hasExports = true;
+        this._options.hasExports = true;
     }

     if (row.entry) row.order = self._entryOrder ++;
@@ -213,6 +220,7 @@ Browserify.prototype.plugin = function (p, opts) {
 Browserify.prototype._createPipeline = function (opts) {
     var self = this;
     if (!opts) opts = {};
+    delete opts.packageCache
     this._mdeps = this._createDeps(opts);
     this._mdeps.on('file', function (file, id) {
         pipeline.emit('file', file, id);

@jkimbo
Copy link

jkimbo commented Oct 17, 2014

@neocoder I came across this problem as well and have found a workaround which is to add the file to the bundle with the expose option as well as requiring it.

e.g.

var b = watchify(bundle(watchify.args));
b.add('./foo.js', { expose: 'foo' });
b.require('./foo.js', { expose: 'foo' });

If you don't have any transforms on your code then just using the require method should suffice. That is another bug in browserify (browserify/browserify#953)

greypants added a commit to vigetlabs/blendid that referenced this issue Jan 3, 2015
Watchify doesn't currently play nice with these options, so for the
meantime, we'll just ignore them during development. Running
`gulp browserify` directly will properly require and externalize.

Related watchify bugs:
browserify/watchify#122
browserify/watchify#72
@jakemhiller
Copy link
Contributor

This is definitely happening with our builds as well. The first build will expose the bundle, and subsequent builds will not.

Tested with browserify 8.0.3 and watchify 2.2.1

@greypants
Copy link

For now, I'm working around it by ignoring expose and require when building with watchify, and only factoring out shared dependencies when my browserify task is run directly (sans watchify). Just means that some modules are duplicated across bundles during development. Not a big deal. They get sorted out for production builds.

if(devMode) {
  // Add watchify args and debug (sourcemaps) option
  _.extend(bundleConfig, watchify.args, { debug: true })
  // A watchify require/external bug that prevents proper recompiling,
  // so (for now) we'll ignore these options during development
  bundleConfig = _.omit(bundleConfig, ['external', 'require'])
}

For reference:
https://github.com/greypants/gulp-starter/blob/e56fedc6110917339f4c0747a5ab3f184b750872/gulp/tasks/browserify.js#L32

I tried @jkimbo's workaround, but couldn't get it working.

@zertosh
Copy link
Member

zertosh commented Mar 25, 2015

@jakemhiller Is this still a problem with browseify 9.0.3 and watchify 2.6.0? More specifically with watchify 2.5.0 - FYI, after that version fullPaths is optional (I think that might be causing you other issues 😜)

@Gijsjan
Copy link

Gijsjan commented Mar 30, 2015

I have the same issue. I'm using npm as a build tool with:

watchify src/index.coffee --outfile build/development/js/main.js --transform coffeeify --extension=".coffee" --transform jadeify --extension=".jade" --require backbone --require jquery --verbose

browserify 9.0.3 and watchify 3.0.0

@zertosh
Copy link
Member

zertosh commented Mar 30, 2015

@Gijsjan: @neocoder's test passes. I'm not sure that you're seeing is the same problem. Can you open a new issue with a reproducible test? Or a repo I can checkout? Anything that'll let me see the problem.

@zertosh zertosh closed this as completed Mar 30, 2015
@zertosh zertosh added bug and removed investigate labels Mar 30, 2015
@pbouzakis
Copy link

That's a shame I've been seeing this issue for a few months. Upgraded to browserify 9 and the issue remains.

It only occurs on modules specified with via require method with an expose opt.

@zertosh
Copy link
Member

zertosh commented Apr 2, 2015

@pbouzakis Can you help me out with some way to reproduce it? Like a test or repo? something?

@pbouzakis
Copy link

Yes I love to. Unfortunately I can easily repro it in my private large git repo.

Tomorrow I'll try to recreate in a simple public repo I can share.

Thanks @zertosh !

@zertosh
Copy link
Member

zertosh commented Apr 2, 2015

@pbouzakis If you want to ping me directly instead of publishing a public repo, my email is my user at gmail.

@pbouzakis
Copy link

@zertosh Really appreciate that. Just sent you an email.

@cheapsteak
Copy link

Can anyone who's previously run into this bug confirm if it's been fixed or still there?

@matthewmueller
Copy link

Still an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants